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

Issue 29439639: Issue 4329 - Add $generichide support (Closed)

Created:
May 17, 2017, 3:33 a.m. by Manish Jethani
Modified:
May 21, 2017, 8:53 p.m.
Reviewers:
Sebastian Noack, kzar
Base URL:
https://hg.adblockplus.org/abp2blocklist
Visibility:
Public.

Description

Implement support for the generichide option Let's say we have these filters: ##.ad blogspot.com##div.textad @@||googleblog.blogspot.com^$elemhide @@||youtube.com^$generichide After this change, the program will generate the following rules: [ { "trigger": { "url-filter": "^https?://", "url-filter-is-case-sensitive": true }, "action": { "type": "css-display-none", "selector": ".ad" } }, { "trigger": { "url-filter": "^https?://([^/]+\\.)?youtube\\.com", "url-filter-is-case-sensitive": true }, "action": { "type": "ignore-previous-rules" } }, { "trigger": { "url-filter": "^https?://([^/:]*\\.)?blogspot\\.com[/:]", "url-filter-is-case-sensitive": true }, "action": { "type": "css-display-none", "selector": "div.textad" } }, { "trigger": { "url-filter": "^https?://([^/]+\\.)?googleblog\\.blogspot\\.com", "url-filter-is-case-sensitive": true }, "action": { "type": "ignore-previous-rules" } } ]

Patch Set 1 #

Total comments: 1

Patch Set 2 : Use separate array for generic selectors #

Total comments: 2

Patch Set 3 : Make matchDomain argument non-optional #

Patch Set 4 : Fix indentation #

Unified diffs Side-by-side diffs Delta from patch set Stats (+61 lines, -25 lines) Patch
M lib/abp2blocklist.js View 1 2 3 3 chunks +44 lines, -23 lines 0 comments Download
M test/abp2blocklist.js View 2 chunks +17 lines, -2 lines 0 comments Download

Messages

Total messages: 10
Manish Jethani
May 17, 2017, 3:33 a.m. (2017-05-17 03:33:20 UTC) #1
Manish Jethani
Patch Set 1
May 17, 2017, 3:40 a.m. (2017-05-17 03:40:17 UTC) #2
Manish Jethani
On 2017/05/17 03:40:17, Manish Jethani wrote: > Patch Set 1 I should add that I ...
May 17, 2017, 3:48 a.m. (2017-05-17 03:48:36 UTC) #3
Sebastian Noack
https://codereview.adblockplus.org/29439639/diff/29439640/lib/abp2blocklist.js File lib/abp2blocklist.js (right): https://codereview.adblockplus.org/29439639/diff/29439640/lib/abp2blocklist.js#newcode422 lib/abp2blocklist.js:422: groupedElemhideFilters.set("^https?://", []); I'm not sure if it is a ...
May 17, 2017, 6:37 a.m. (2017-05-17 06:37:10 UTC) #4
Manish Jethani
Patch Set 2: Use separate array for generic selectors https://codereview.adblockplus.org/29439639/diff/29440586/lib/abp2blocklist.js File lib/abp2blocklist.js (right): https://codereview.adblockplus.org/29439639/diff/29440586/lib/abp2blocklist.js#newcode358 lib/abp2blocklist.js:358: ...
May 17, 2017, 2:54 p.m. (2017-05-17 14:54:04 UTC) #5
kzar
https://codereview.adblockplus.org/29439639/diff/29440586/lib/abp2blocklist.js File lib/abp2blocklist.js (right): https://codereview.adblockplus.org/29439639/diff/29440586/lib/abp2blocklist.js#newcode464 lib/abp2blocklist.js:464: addCSSRules(rules, genericSelectors); Just pass matchDomain of ^https?:// here, then ...
May 19, 2017, 12:12 p.m. (2017-05-19 12:12:22 UTC) #6
Manish Jethani
Patch Set 3: Make matchDomain argument non-optional
May 20, 2017, 12:16 a.m. (2017-05-20 00:16:52 UTC) #7
Manish Jethani
Patch Set 4: Fix indentation
May 20, 2017, 12:21 a.m. (2017-05-20 00:21:43 UTC) #8
Sebastian Noack
LGTM
May 20, 2017, 6:20 a.m. (2017-05-20 06:20:07 UTC) #9
kzar
May 20, 2017, 10:21 a.m. (2017-05-20 10:21:51 UTC) #10
LGTM

Powered by Google App Engine
This is Rietveld