Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

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

Created:
March 22, 2018, 4:09 p.m. by Manish Jethani
Modified:
March 31, 2018, 2:26 p.m.
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
March 22, 2018, 4:09 p.m. (2018-03-22 16:09:55 UTC) #1
Manish Jethani
Patch Set 1 See description.
March 22, 2018, 4:17 p.m. (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 ...
March 22, 2018, 4:31 p.m. (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) ...
March 23, 2018, 2:58 p.m. (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) ...
March 23, 2018, 4:56 p.m. (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) ...
March 24, 2018, 1:02 a.m. (2018-03-24 01:02:14 UTC) #6
Manish Jethani
March 31, 2018, 2:25 p.m. (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.

Powered by Google App Engine
This is Rietveld