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

Issue 29559743: Issue 5650 - Apply emulation filters to shadow DOMs

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 weeks ago by Manish Jethani
Modified:
9 hours, 34 minutes ago
CC:
kzar
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Patch Set 1 #

Total comments: 6

Patch Set 2 : Prevent author code from intercepting shadowAttached event #

Total comments: 7

Patch Set 3 : Find shadow roots semi-asynchronously #

Total comments: 4

Patch Set 4 : Remove unused node argument #

Total comments: 11

Patch Set 5 : Terminate loop #

Patch Set 6 : Use Node.contains #

Patch Set 7 : Take event type as a parameter #

Total comments: 4

Patch Set 8 : Optimize removeRedundantNodes based on node order #

Total comments: 5

Patch Set 9 : Run code only on if Shadow DOM is supported #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+148 lines, -16 lines) Patch
M lib/content/elemHideEmulation.js View 1 2 3 4 5 6 7 8 6 chunks +146 lines, -15 lines 2 comments Download
M test/browser/elemHideEmulation.js View 1 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 30
Manish Jethani
3 weeks ago (2017-09-29 23:21:34 UTC) #1
Manish Jethani
Patch Set 1 For -abp-has and -abp-contains we need to be able to peek into ...
3 weeks ago (2017-09-29 23:31:30 UTC) #2
Manish Jethani
https://codereview.adblockplus.org/29559743/diff/29559744/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29559744/lib/content/elemHideEmulation.js#newcode604 lib/content/elemHideEmulation.js:604: for (let child of node.children) We could use a ...
3 weeks ago (2017-09-29 23:43:56 UTC) #3
Manish Jethani
Patch Set 2: Prevent author code from intercepting shadowAttached event https://codereview.adblockplus.org/29559743/diff/29560555/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29560555/lib/content/elemHideEmulation.js#newcode308 ...
2 weeks, 6 days ago (2017-09-30 12:29:01 UTC) #4
lainverse
https://codereview.adblockplus.org/29559743/diff/29560555/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29560555/lib/content/elemHideEmulation.js#newcode585 lib/content/elemHideEmulation.js:585: event.stopImmediatePropagation(); On 2017/09/30 12:29:01, Manish Jethani wrote: > Prevent ...
2 weeks, 4 days ago (2017-10-02 14:11:55 UTC) #5
Manish Jethani
https://codereview.adblockplus.org/29559743/diff/29560555/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29560555/lib/content/elemHideEmulation.js#newcode585 lib/content/elemHideEmulation.js:585: event.stopImmediatePropagation(); On 2017/10/02 14:11:55, lainverse wrote: > On 2017/09/30 ...
2 weeks, 4 days ago (2017-10-02 14:28:11 UTC) #6
lainverse
https://codereview.adblockplus.org/29559743/diff/29560555/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29560555/lib/content/elemHideEmulation.js#newcode585 lib/content/elemHideEmulation.js:585: event.stopImmediatePropagation(); On 2017/10/02 14:28:11, Manish Jethani wrote: > On ...
2 weeks, 4 days ago (2017-10-02 16:05:39 UTC) #7
Manish Jethani
https://codereview.adblockplus.org/29559743/diff/29560555/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29560555/lib/content/elemHideEmulation.js#newcode585 lib/content/elemHideEmulation.js:585: event.stopImmediatePropagation(); On 2017/10/02 16:05:39, lainverse wrote: > [...] > ...
2 weeks, 4 days ago (2017-10-02 16:12:31 UTC) #8
Manish Jethani
I'll file an issue with Chromium to allow extensions to access closed shadow roots. Meanwhile, ...
1 week ago (2017-10-14 00:52:37 UTC) #9
Sebastian Noack
On 2017/10/14 00:52:37, Manish Jethani wrote: > I'll file an issue with Chromium to allow ...
3 days, 7 hours ago (2017-10-17 21:44:16 UTC) #10
hub
This sounds like a good idea.
2 days, 12 hours ago (2017-10-18 16:38:09 UTC) #11
Manish Jethani
Patch Set 3: Find shadow roots semi-asynchronously https://codereview.adblockplus.org/29559743/diff/29582707/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29582707/lib/content/elemHideEmulation.js#newcode149 lib/content/elemHideEmulation.js:149: function removeRedundantNodes(nodes) ...
2 days, 5 hours ago (2017-10-18 23:08:00 UTC) #12
Manish Jethani
Patch Set 4: Remove unused node argument
2 days, 5 hours ago (2017-10-18 23:09:04 UTC) #13
Sebastian Noack
https://codereview.adblockplus.org/29559743/diff/29582710/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29582710/lib/content/elemHideEmulation.js#newcode168 lib/content/elemHideEmulation.js:168: nodeInfo.redundant = true; Why don't you just keep track ...
2 days, 4 hours ago (2017-10-19 00:43:08 UTC) #14
Manish Jethani
Patch Set 5: Terminate loop https://codereview.adblockplus.org/29559743/diff/29582710/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29582710/lib/content/elemHideEmulation.js#newcode168 lib/content/elemHideEmulation.js:168: nodeInfo.redundant = true; On ...
2 days, 2 hours ago (2017-10-19 02:13:45 UTC) #15
Manish Jethani
https://codereview.adblockplus.org/29559743/diff/29582710/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29582710/lib/content/elemHideEmulation.js#newcode168 lib/content/elemHideEmulation.js:168: nodeInfo.redundant = true; On 2017/10/19 02:13:44, Manish Jethani wrote: ...
2 days, 2 hours ago (2017-10-19 02:19:06 UTC) #16
Sebastian Noack
https://codereview.adblockplus.org/29559743/diff/29582710/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29582710/lib/content/elemHideEmulation.js#newcode168 lib/content/elemHideEmulation.js:168: nodeInfo.redundant = true; On 2017/10/19 02:19:06, Manish Jethani wrote: ...
2 days ago (2017-10-19 03:51:02 UTC) #17
Manish Jethani
https://codereview.adblockplus.org/29559743/diff/29582710/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29582710/lib/content/elemHideEmulation.js#newcode168 lib/content/elemHideEmulation.js:168: nodeInfo.redundant = true; On 2017/10/19 03:51:01, Sebastian Noack wrote: ...
1 day, 18 hours ago (2017-10-19 09:57:14 UTC) #18
lainverse
Maybe something like this: // parentNode crawler function removeRedundantNodes(nodes) { let link, redundant = []; ...
1 day, 14 hours ago (2017-10-19 14:01:39 UTC) #19
lainverse
// parentNode crawler function removeRedundantNodes(nodes) { let link, redundant = []; for (let node of ...
1 day, 14 hours ago (2017-10-19 14:02:01 UTC) #20
lainverse
// parent.contains(child) when nodes is an Array let removeRedundantNodes = nodes => nodes.filter( node => ...
1 day, 13 hours ago (2017-10-19 15:33:52 UTC) #21
Sebastian Noack
https://codereview.adblockplus.org/29559743/diff/29582710/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29582710/lib/content/elemHideEmulation.js#newcode168 lib/content/elemHideEmulation.js:168: nodeInfo.redundant = true; On 2017/10/19 09:57:14, Manish Jethani wrote: ...
1 day, 10 hours ago (2017-10-19 18:35:40 UTC) #22
Manish Jethani
Patch Set 6: Use Node.contains https://codereview.adblockplus.org/29559743/diff/29582710/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29582710/lib/content/elemHideEmulation.js#newcode168 lib/content/elemHideEmulation.js:168: nodeInfo.redundant = true; On ...
1 day, 5 hours ago (2017-10-19 23:14:59 UTC) #23
Manish Jethani
Patch Set 7: Take event type as a parameter https://codereview.adblockplus.org/29559743/diff/29583631/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29583631/lib/content/elemHideEmulation.js#newcode348 lib/content/elemHideEmulation.js:348: ...
1 day, 5 hours ago (2017-10-19 23:44:03 UTC) #24
Manish Jethani
Patch Set 8: Optimize removeRedundantNodes based on node order A node always appears before any ...
14 hours, 7 minutes ago (2017-10-20 14:39:10 UTC) #25
lainverse
https://codereview.adblockplus.org/29559743/diff/29584630/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29584630/lib/content/elemHideEmulation.js#newcode679 lib/content/elemHideEmulation.js:679: if (node instanceof Element) Actually if nodes are always ...
13 hours, 10 minutes ago (2017-10-20 15:37:08 UTC) #26
hub
https://codereview.adblockplus.org/29559743/diff/29584630/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29584630/lib/content/elemHideEmulation.js#newcode48 lib/content/elemHideEmulation.js:48: let newSelector = node instanceof ShadowRoot ? ":host" : ...
13 hours, 9 minutes ago (2017-10-20 15:37:57 UTC) #27
Manish Jethani
Patch Set 9: Run code only on if Shadow DOM is supported https://codereview.adblockplus.org/29559743/diff/29584630/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js ...
12 hours, 30 minutes ago (2017-10-20 16:16:42 UTC) #28
lainverse
https://codereview.adblockplus.org/29559743/diff/29584630/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29559743/diff/29584630/lib/content/elemHideEmulation.js#newcode679 lib/content/elemHideEmulation.js:679: if (node instanceof Element) > While it is better ...
11 hours, 9 minutes ago (2017-10-20 17:37:36 UTC) #29
hub
9 hours, 34 minutes ago (2017-10-20 19:12:51 UTC) #30
https://codereview.adblockplus.org/29559743/diff/29584695/lib/content/elemHid...
File lib/content/elemHideEmulation.js (right):

https://codereview.adblockplus.org/29559743/diff/29584695/lib/content/elemHid...
lib/content/elemHideEmulation.js:679: if (typeof ShadowRoot != "undefined")
On 2017/10/20 16:16:42, Manish Jethani wrote:
> Of course we only have to run this code on Chrome.

Maybe we should set a different mutation observer based on this as the observer
is called quite often.
Sign in to reply to this message.

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