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

Issue 29493648: Issue 5436 - Allow relative selectors in :-abp-has() (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 months, 3 weeks ago by hub
Modified:
3 months, 4 weeks ago
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Description

Issue 5436 - Allow relative selectors in :-abp-has()

Patch Set 1 #

Patch Set 2 : Forgot a small cleanup change #

Total comments: 8

Patch Set 3 : Updated with review comment. Added other test. #

Patch Set 4 : Remove unecessary changes #

Total comments: 6

Patch Set 5 : Updated with review feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+69 lines, -16 lines) Patch
M chrome/content/elemHideEmulation.js View 1 2 3 4 2 chunks +14 lines, -2 lines 0 comments Download
M test/browser/elemHideEmulation.js View 1 2 3 4 1 chunk +55 lines, -14 lines 0 comments Download

Messages

Total messages: 10
hub
4 months, 3 weeks ago (2017-07-20 19:18:13 UTC) #1
hub
Currently based off the adblockpluschrome-1.13.3. This must be checked onto master when the branch patches ...
4 months, 3 weeks ago (2017-07-20 19:19:08 UTC) #2
hub
https://codereview.adblockplus.org/29493648/diff/29493652/test/browser/elemHideEmulation.js File test/browser/elemHideEmulation.js (right): https://codereview.adblockplus.org/29493648/diff/29493652/test/browser/elemHideEmulation.js#newcode395 test/browser/elemHideEmulation.js:395: }; This is the test that was removed as ...
4 months, 3 weeks ago (2017-07-20 19:21:42 UTC) #3
Wladimir Palant
https://codereview.adblockplus.org/29493648/diff/29493652/chrome/content/elemHideEmulation.js File chrome/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29493648/diff/29493652/chrome/content/elemHideEmulation.js#newcode232 chrome/content/elemHideEmulation.js:232: selector = ":scope" + selector; Nice one, I didn't ...
4 months ago (2017-08-10 13:36:08 UTC) #4
Wladimir Palant
https://codereview.adblockplus.org/29493648/diff/29493652/test/browser/elemHideEmulation.js File test/browser/elemHideEmulation.js (right): https://codereview.adblockplus.org/29493648/diff/29493652/test/browser/elemHideEmulation.js#newcode394 test/browser/elemHideEmulation.js:394: runTestPseudoClassHasSelectorWithHasAndWithSuffixSibling(test, "div:-abp-has(:-abp-has(> div.inside)) + div > div"); It's probably ...
4 months ago (2017-08-10 13:39:48 UTC) #5
Wladimir Palant
https://codereview.adblockplus.org/29493648/diff/29493652/chrome/content/elemHideEmulation.js File chrome/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29493648/diff/29493652/chrome/content/elemHideEmulation.js#newcode194 chrome/content/elemHideEmulation.js:194: const relativeSelectorRegexp = /^[\s>+~]/; \s doesn't belong in here. ...
4 months ago (2017-08-10 13:42:17 UTC) #6
hub
Updated patch. This is based on master. https://codereview.adblockplus.org/29493648/diff/29493652/chrome/content/elemHideEmulation.js File chrome/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29493648/diff/29493652/chrome/content/elemHideEmulation.js#newcode194 chrome/content/elemHideEmulation.js:194: const relativeSelectorRegexp ...
4 months ago (2017-08-14 14:25:33 UTC) #7
Wladimir Palant
https://codereview.adblockplus.org/29493648/diff/29515588/chrome/content/elemHideEmulation.js File chrome/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29493648/diff/29515588/chrome/content/elemHideEmulation.js#newcode229 chrome/content/elemHideEmulation.js:229: // :scope isn't supported on Edge. It still is ...
4 months ago (2017-08-16 08:17:16 UTC) #8
hub
https://codereview.adblockplus.org/29493648/diff/29515588/chrome/content/elemHideEmulation.js File chrome/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29493648/diff/29515588/chrome/content/elemHideEmulation.js#newcode229 chrome/content/elemHideEmulation.js:229: // :scope isn't supported on Edge. It still is ...
4 months ago (2017-08-16 15:30:46 UTC) #9
Wladimir Palant
3 months, 4 weeks ago (2017-08-17 08:41:01 UTC) #10
LGTM
Sign in to reply to this message.

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