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

Issue 29585589: Issue 6423 - Observe only relevant mutation types (Closed)

Created:
Oct. 22, 2017, 10:51 p.m. by Manish Jethani
Modified:
Feb. 28, 2018, 10:42 a.m.
Reviewers:
kzar, hub
CC:
sergei
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Patch Set 1 #

Total comments: 3

Patch Set 2 : Account for nested selectors #

Total comments: 2

Patch Set 3 : Account for ID and class selectors #

Total comments: 2

Patch Set 4 : Rebase #

Patch Set 5 : Add brackets #

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

Messages

Total messages: 11
Manish Jethani
Oct. 22, 2017, 10:51 p.m. (2017-10-22 22:51:31 UTC) #1
Manish Jethani
Patch Set 1 We don't have to observe all mutation types all the time. https://codereview.adblockplus.org/29585589/diff/29585590/lib/content/elemHideEmulation.js ...
Oct. 22, 2017, 10:58 p.m. (2017-10-22 22:58:14 UTC) #2
Manish Jethani
Patch Set 2: Account for nested selectors The first one clearly was wrong and did ...
Oct. 23, 2017, 12:12 a.m. (2017-10-23 00:12:10 UTC) #3
hub
https://codereview.adblockplus.org/29585589/diff/29585592/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29585589/diff/29585592/lib/content/elemHideEmulation.js#newcode149 lib/content/elemHideEmulation.js:149: this.maybeDependsOnAttributes = /\[.+\]/.test(selector); What about the changes of `class` ...
Oct. 23, 2017, 1:18 p.m. (2017-10-23 13:18:56 UTC) #4
Manish Jethani
Patch Set 3: Account for ID and class selectors https://codereview.adblockplus.org/29585589/diff/29585592/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29585589/diff/29585592/lib/content/elemHideEmulation.js#newcode149 lib/content/elemHideEmulation.js:149: ...
Oct. 23, 2017, 10:52 p.m. (2017-10-23 22:52:08 UTC) #5
Manish Jethani
What about this one, does it look OK?
Feb. 21, 2018, 4:42 p.m. (2018-02-21 16:42:34 UTC) #6
hub
https://codereview.adblockplus.org/29585589/diff/29586713/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29585589/diff/29586713/lib/content/elemHideEmulation.js#newcode332 lib/content/elemHideEmulation.js:332: selector.dependsOnStyles I'm always forgetting operator precedence. Should there be ...
Feb. 21, 2018, 6:12 p.m. (2018-02-21 18:12:27 UTC) #7
Manish Jethani
Patch Set 5: Add brackets https://codereview.adblockplus.org/29585589/diff/29586713/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29585589/diff/29586713/lib/content/elemHideEmulation.js#newcode332 lib/content/elemHideEmulation.js:332: selector.dependsOnStyles On 2018/02/21 18:12:27, ...
Feb. 22, 2018, 7:14 a.m. (2018-02-22 07:14:33 UTC) #8
Manish Jethani
I have filed a Trac issue now: https://issues.adblockplus.org/ticket/6423
Feb. 26, 2018, 1:59 p.m. (2018-02-26 13:59:28 UTC) #9
hub
as it is, LGTM But please get a signoff from the module owner.
Feb. 26, 2018, 3:38 p.m. (2018-02-26 15:38:41 UTC) #10
kzar
Feb. 26, 2018, 5:19 p.m. (2018-02-26 17:19:51 UTC) #11
Hubert knows this code better than me, if he's happy please go ahead.

Powered by Google App Engine
This is Rietveld