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

Issue 29464708: Issue 5314 - Allow hide emulation filters to be with a plain selector (Closed)

Created:
June 14, 2017, 3:18 a.m. by hub
Modified:
June 19, 2017, 2:03 p.m.
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Description

Issue 5314 - Allow hide emulation filters to be with a plain selector

Patch Set 1 #

Total comments: 2

Patch Set 2 : Updated the logic #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -7 lines) Patch
M chrome/content/elemHideEmulation.js View 1 2 chunks +4 lines, -1 line 0 comments Download
M lib/filterClasses.js View 1 chunk +0 lines, -4 lines 0 comments Download
M test/filterClasses.js View 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 6
hub
June 14, 2017, 3:18 a.m. (2017-06-14 03:18:58 UTC) #1
Sebastian Noack
LGTM!
June 14, 2017, 4:49 a.m. (2017-06-14 04:49:30 UTC) #2
Wladimir Palant
https://codereview.adblockplus.org/29464708/diff/29464709/chrome/content/elemHideEmulation.js File chrome/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29464708/diff/29464709/chrome/content/elemHideEmulation.js#newcode380 chrome/content/elemHideEmulation.js:380: if (pattern.selectors.some(s => s.hideWithStyleSheet)) This logic reversal is wrong ...
June 19, 2017, 8:28 a.m. (2017-06-19 08:28:32 UTC) #3
hub
updated patch. https://codereview.adblockplus.org/29464708/diff/29464709/chrome/content/elemHideEmulation.js File chrome/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29464708/diff/29464709/chrome/content/elemHideEmulation.js#newcode380 chrome/content/elemHideEmulation.js:380: if (pattern.selectors.some(s => s.hideWithStyleSheet)) On 2017/06/19 08:28:32, ...
June 19, 2017, 1:37 p.m. (2017-06-19 13:37:15 UTC) #4
Wladimir Palant
LGTM
June 19, 2017, 1:44 p.m. (2017-06-19 13:44:24 UTC) #5
Sebastian Noack
June 19, 2017, 1:59 p.m. (2017-06-19 13:59:19 UTC) #6
On 2017/06/19 13:44:24, Wladimir Palant wrote:
> LGTM

LGTM

Powered by Google App Engine
This is Rietveld