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

Issue 6439460933730304: Issue 616 - Add tests for $generichide and $genericblock (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 6 months ago by kzar
Modified:
3 years, 11 months ago
CC:
Wladimir Palant, Sebastian Noack
Visibility:
Public.

Description

Issue 616 - Add tests for $generichide and $genericblock Depends on these changes: http://codereview.adblockplus.org/5840485868371968/ http://codereview.adblockplus.org/5991150368325632/

Patch Set 1 #

Total comments: 7

Patch Set 2 : Removed comment that I had forgotten about. #

Patch Set 3 : Rebased onto typeMask changes #

Patch Set 4 : Rebased onto Wladimir's test fixes and added test for genericblock sitekey interaction #

Patch Set 5 : Fixed contentpolicy tests with recent changes to underlying logic #

Total comments: 8

Patch Set 6 : Addressed Felix's feedback #

Total comments: 2

Patch Set 7 : Slight improvement #

Total comments: 12

Patch Set 8 : Addressed The Professor's feedback! #

Patch Set 9 : Couple more tweaks #

Unified diffs Side-by-side diffs Delta from patch set Stats (+119 lines, -80 lines) Patch
M chrome/content/tests/elemhide.js View 1 2 3 4 5 6 7 8 2 chunks +17 lines, -3 lines 0 comments Download
M chrome/content/tests/filterClasses.js View 1 2 3 4 5 6 7 2 chunks +6 lines, -2 lines 0 comments Download
M chrome/content/tests/matcher.js View 1 2 3 4 5 6 7 8 3 chunks +83 lines, -72 lines 0 comments Download
M chrome/content/tests/policy.js View 1 2 3 4 5 3 chunks +13 lines, -3 lines 0 comments Download

Messages

Total messages: 18
kzar
Patch Set 1 http://codereview.adblockplus.org/6439460933730304/diff/5629499534213120/chrome/content/tests/matcher.js File chrome/content/tests/matcher.js (right): http://codereview.adblockplus.org/6439460933730304/diff/5629499534213120/chrome/content/tests/matcher.js#newcode101 chrome/content/tests/matcher.js:101: { Most of this change is ...
4 years, 6 months ago (2015-03-19 11:03:22 UTC) #1
Sebastian Noack
http://codereview.adblockplus.org/6439460933730304/diff/5629499534213120/chrome/content/tests/elemhide.js File chrome/content/tests/elemhide.js (right): http://codereview.adblockplus.org/6439460933730304/diff/5629499534213120/chrome/content/tests/elemhide.js#newcode151 chrome/content/tests/elemhide.js:151: if (filter.substr(0, 2) == "@@") How about using two ...
4 years, 6 months ago (2015-03-19 11:58:49 UTC) #2
kzar
Patch Set 2 : Removed comment that I had forgotten about. http://codereview.adblockplus.org/6439460933730304/diff/5629499534213120/chrome/content/tests/elemhide.js File chrome/content/tests/elemhide.js (right): ...
4 years, 6 months ago (2015-03-19 16:24:26 UTC) #3
Sebastian Noack
LGTM http://codereview.adblockplus.org/6439460933730304/diff/5629499534213120/chrome/content/tests/elemhide.js File chrome/content/tests/elemhide.js (right): http://codereview.adblockplus.org/6439460933730304/diff/5629499534213120/chrome/content/tests/elemhide.js#newcode151 chrome/content/tests/elemhide.js:151: if (filter.substr(0, 2) == "@@") On 2015/03/19 16:24:26, ...
4 years, 6 months ago (2015-03-20 09:56:12 UTC) #4
kzar
Patch Set 3 : Rebased onto typeMask changes
4 years, 2 months ago (2015-07-14 16:50:30 UTC) #5
kzar
Patch Set 4 : Rebased onto Wladimir's test fixes and added test for genericblock sitekey ...
4 years ago (2015-08-25 17:50:11 UTC) #6
kzar
Patch Set 5 : Fixed contentpolicy tests with recent changes to underlying logic
4 years ago (2015-09-05 14:42:28 UTC) #7
Sebastian Noack
LGTM
3 years, 12 months ago (2015-09-21 14:28:13 UTC) #8
Felix Dahlke
https://codereview.adblockplus.org/6439460933730304/diff/29325968/chrome/content/tests/elemhide.js File chrome/content/tests/elemhide.js (right): https://codereview.adblockplus.org/6439460933730304/diff/29325968/chrome/content/tests/elemhide.js#newcode151 chrome/content/tests/elemhide.js:151: if (filter.substr(0, 2) == "@@") How I understand it, ...
3 years, 11 months ago (2015-09-28 18:24:08 UTC) #9
kzar
Patch Set 6 : Addressed Felix's feedback https://codereview.adblockplus.org/6439460933730304/diff/29325968/chrome/content/tests/elemhide.js File chrome/content/tests/elemhide.js (right): https://codereview.adblockplus.org/6439460933730304/diff/29325968/chrome/content/tests/elemhide.js#newcode151 chrome/content/tests/elemhide.js:151: if (filter.substr(0, ...
3 years, 11 months ago (2015-09-29 11:07:02 UTC) #10
Felix Dahlke
Almost there, wouldn't even insist on that last change. https://codereview.adblockplus.org/6439460933730304/diff/29328689/chrome/content/tests/elemhide.js File chrome/content/tests/elemhide.js (right): https://codereview.adblockplus.org/6439460933730304/diff/29328689/chrome/content/tests/elemhide.js#newcode153 chrome/content/tests/elemhide.js:153: ...
3 years, 11 months ago (2015-10-01 11:53:41 UTC) #11
kzar
Patch Set 7 : Slight improvement https://codereview.adblockplus.org/6439460933730304/diff/29328689/chrome/content/tests/elemhide.js File chrome/content/tests/elemhide.js (right): https://codereview.adblockplus.org/6439460933730304/diff/29328689/chrome/content/tests/elemhide.js#newcode153 chrome/content/tests/elemhide.js:153: if (filter instanceof ...
3 years, 11 months ago (2015-10-01 12:16:53 UTC) #12
Felix Dahlke
LGTM Sebastian is unavailable for the rest of this week and LGTMd an earlier patch ...
3 years, 11 months ago (2015-10-01 13:37:28 UTC) #13
Thomas Greiner
https://codereview.adblockplus.org/6439460933730304/diff/29328786/chrome/content/tests/elemhide.js File chrome/content/tests/elemhide.js (right): https://codereview.adblockplus.org/6439460933730304/diff/29328786/chrome/content/tests/elemhide.js#newcode110 chrome/content/tests/elemhide.js:110: [["#div(test1)", "@@localhost$generichide"], ["visible", "visible"]], The simplified element hiding filter ...
3 years, 11 months ago (2015-10-01 16:12:05 UTC) #14
kzar
Patch Set 8 : Addressed The Professor's feedback! https://codereview.adblockplus.org/6439460933730304/diff/29328786/chrome/content/tests/elemhide.js File chrome/content/tests/elemhide.js (right): https://codereview.adblockplus.org/6439460933730304/diff/29328786/chrome/content/tests/elemhide.js#newcode110 chrome/content/tests/elemhide.js:110: [["#div(test1)", ...
3 years, 11 months ago (2015-10-02 10:19:29 UTC) #15
kzar
Patch Set 9 : Couple more tweaks
3 years, 11 months ago (2015-10-02 10:40:04 UTC) #16
Thomas Greiner
LGTM https://codereview.adblockplus.org/6439460933730304/diff/29328786/chrome/content/tests/filterClasses.js File chrome/content/tests/filterClasses.js (right): https://codereview.adblockplus.org/6439460933730304/diff/29328786/chrome/content/tests/filterClasses.js#newcode95 chrome/content/tests/filterClasses.js:95: addProperty("contentType", 0x7FFFFFFF & ~(RegExpFilter.typeMap.DOCUMENT | RegExpFilter.typeMap.ELEMHIDE | RegExpFilter.typeMap.POPUP ...
3 years, 11 months ago (2015-10-02 13:55:47 UTC) #17
Felix Dahlke
3 years, 11 months ago (2015-10-02 15:52:03 UTC) #18
LGTM
Sign in to reply to this message.

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