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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 11 months ago by hub
Modified:
2 years, 11 months ago
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
2 years, 11 months ago (2017-06-14 03:18:58 UTC) #1
Sebastian Noack
LGTM!
2 years, 11 months ago (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 ...
2 years, 11 months ago (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, ...
2 years, 11 months ago (2017-06-19 13:37:15 UTC) #4
Wladimir Palant
LGTM
2 years, 11 months ago (2017-06-19 13:44:24 UTC) #5
Sebastian Noack
2 years, 11 months ago (2017-06-19 13:59:19 UTC) #6
On 2017/06/19 13:44:24, Wladimir Palant wrote:
> LGTM

LGTM
Sign in to reply to this message.

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