Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(211)

Issue 29730630: Issue 6437 - Skip DOM-only patterns on initial load (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 8 months ago by Manish Jethani
Modified:
1 year, 8 months ago
Reviewers:
hub
CC:
kzar
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Description

This will be especially useful after https://codereview.adblockplus.org/29714601/ Ads are almost never part of the original HTML, they're loaded dynamically via JS or by some other means. In that case, particularly for the top-level frame, there's no need to traverse the DOM for DOM-only patterns until there's a mutation. This way when a mutation occurs we can target the newly added nodes quickly instead of waiting for a full traversal. If we do this, ads will be hidden immediately instead of appearing momentarily before disappearing as they do now.

Patch Set 1 #

Patch Set 2 : Check if it's the top frame #

Patch Set 3 : Fix unrelated typo in comments #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -3 lines) Patch
M lib/content/elemHideEmulation.js View 1 2 3 chunks +12 lines, -3 lines 3 comments Download

Messages

Total messages: 7
Manish Jethani
1 year, 8 months ago (2018-03-22 16:09:55 UTC) #1
Manish Jethani
Patch Set 1 See description.
1 year, 8 months ago (2018-03-22 16:17:45 UTC) #2
Manish Jethani
Patch Set 2: Check if it's the top frame Patch Set 3: Fix unrelated typo ...
1 year, 8 months ago (2018-03-22 16:31:29 UTC) #3
hub
https://codereview.adblockplus.org/29730630/diff/29730635/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29730630/diff/29730635/lib/content/elemHideEmulation.js#newcode463 lib/content/elemHideEmulation.js:463: if (typeof top != "undefined" && document.defaultView == top) ...
1 year, 8 months ago (2018-03-23 14:58:59 UTC) #4
Manish Jethani
https://codereview.adblockplus.org/29730630/diff/29730635/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29730630/diff/29730635/lib/content/elemHideEmulation.js#newcode463 lib/content/elemHideEmulation.js:463: if (typeof top != "undefined" && document.defaultView == top) ...
1 year, 8 months ago (2018-03-23 16:56:46 UTC) #5
hub
https://codereview.adblockplus.org/29730630/diff/29730635/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29730630/diff/29730635/lib/content/elemHideEmulation.js#newcode463 lib/content/elemHideEmulation.js:463: if (typeof top != "undefined" && document.defaultView == top) ...
1 year, 8 months ago (2018-03-24 01:02:14 UTC) #6
Manish Jethani
1 year, 8 months ago (2018-03-31 14:25:46 UTC) #7
I'm dropping this for now. I can't find a good way to do this without further
complicating the code. Ideally we would skip the DOM-only patterns when the
document's "readyState" is "loading" and then do the DOM-only patterns only (but
not any other patterns!) when the state changes to "complete". I'll see if I can
come back to do this, but I'm closing this for now.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5