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

Issue 29737558: Issue 6538, 6781 - Implement support for snippet filters (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 months, 2 weeks ago by Manish Jethani
Modified:
1 month ago
CC:
mapx
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Patch Set 1 #

Patch Set 2 : Check for null and blank string instead #

Patch Set 3 : Rebase #

Patch Set 4 : Make snippet filters similar to element hiding filters #

Total comments: 6

Patch Set 5 : Remove RegExpFilter.typeMap.SNIPPET #

Patch Set 6 : Fix normalization and add more tests #

Patch Set 7 : Rebase #

Patch Set 8 : Return code directly from getScriptsForDomain #

Patch Set 9 : Update tests #

Total comments: 2

Patch Set 10 : Rebase #

Patch Set 11 : Fix rebase #

Patch Set 12 : Add filter to known filters #

Patch Set 13 : Rebase, rename to ScriptFilter, ignore element hiding exceptions #

Patch Set 14 : Rebase #

Patch Set 15 : Rebase #

Total comments: 2

Patch Set 16 : Rename ScriptFilter to ContentFilter #

Patch Set 17 : Make ElemHideBase.selector a getter #

Total comments: 5

Patch Set 18 : Change ElemHideBase.fromText to ContentFilter.fromText #

Patch Set 19 : Rebase #

Patch Set 20 : Fix JSDoc comment #

Total comments: 27

Patch Set 21 : Rename script to body in ContentFilter #

Total comments: 2

Patch Set 22 : Improve comment formatting #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+320 lines, -37 lines) Patch
M lib/filterClasses.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 5 chunks +91 lines, -32 lines 2 comments Download
M lib/filterListener.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 4 chunks +8 lines, -1 line 0 comments Download
A lib/snippets.js View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +77 lines, -0 lines 0 comments Download
M test/filterClasses.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 7 chunks +37 lines, -4 lines 0 comments Download
A test/snippets.js View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +107 lines, -0 lines 0 comments Download

Messages

Total messages: 34
Manish Jethani
4 months, 2 weeks ago (2018-03-30 20:19:00 UTC) #1
kzar
(Adding Mapx to Cc at his request.)
4 months, 1 week ago (2018-04-03 08:59:47 UTC) #2
Manish Jethani
Patch Set 2: Check for null and blank string instead
4 months, 1 week ago (2018-04-03 10:46:30 UTC) #3
sergei
Could you please add tests?
4 months ago (2018-04-11 09:57:18 UTC) #4
Manish Jethani
Patch Set 4: Make snippet filters similar to element hiding filters I've updated the implementation ...
3 months, 2 weeks ago (2018-04-26 12:21:52 UTC) #5
Manish Jethani
https://codereview.adblockplus.org/29737558/diff/29761590/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29737558/diff/29761590/lib/filterClasses.js#newcode872 lib/filterClasses.js:872: SNIPPET: 0x8000000, I had to take this slot since ...
3 months, 2 weeks ago (2018-04-26 13:19:17 UTC) #6
hub
https://codereview.adblockplus.org/29737558/diff/29768675/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29737558/diff/29768675/lib/filterClasses.js#newcode136 lib/filterClasses.js:136: return new SnippetFilter(text, match[1], match[3]); Here you shouldn't return, ...
3 months, 1 week ago (2018-05-03 22:59:38 UTC) #7
Manish Jethani
Patch Set 10: Rebase Patch Set 11: Fix rebase Patch Set 12: Add filter to ...
3 months, 1 week ago (2018-05-07 19:03:28 UTC) #8
Manish Jethani
Patch Set 13: Rebase, rename to ScriptFilter, ignore element hiding exceptions I've updated the implementation ...
2 months, 3 weeks ago (2018-05-23 03:59:48 UTC) #9
Manish Jethani
Patch Set 14: Rebase ElemHideBase.selectorDomains is gone, as has ScriptFilter.scriptDomains now. Still not sure about ...
2 months ago (2018-06-11 10:07:50 UTC) #10
Manish Jethani
Patch Set 15: Rebase Another rebase.
1 month, 3 weeks ago (2018-06-19 13:00:49 UTC) #11
Manish Jethani
Please see this comment: https://issues.adblockplus.org/ticket/6538#comment:77 Can we start with the code reviews now? I have ...
1 month, 3 weeks ago (2018-06-21 21:42:59 UTC) #12
hub
https://codereview.adblockplus.org/29737558/diff/29810559/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29737558/diff/29810559/lib/filterClasses.js#newcode999 lib/filterClasses.js:999: function ScriptFilter(text, domains, script) I'm not convinced with the ...
1 month, 3 weeks ago (2018-06-22 20:45:40 UTC) #13
Manish Jethani
Patch Set 16: Rename ScriptFilter to ContentFilter https://codereview.adblockplus.org/29737558/diff/29810559/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29737558/diff/29810559/lib/filterClasses.js#newcode999 lib/filterClasses.js:999: function ScriptFilter(text, ...
1 month, 3 weeks ago (2018-06-23 18:24:09 UTC) #14
Manish Jethani
Patch Set 17: Make ElemHideBase.selector a getter https://codereview.adblockplus.org/29737558/diff/29816558/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29737558/diff/29816558/lib/filterClasses.js#newcode1040 lib/filterClasses.js:1040: get selector() ...
1 month, 2 weeks ago (2018-06-26 19:28:16 UTC) #15
hub
https://codereview.adblockplus.org/29737558/diff/29816558/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29737558/diff/29816558/lib/filterClasses.js#newcode138 lib/filterClasses.js:138: filter = ElemHideBase.fromText(text, match[1], match[2], match[3]); I'm wonder if ...
1 month, 1 week ago (2018-07-05 08:40:15 UTC) #16
Manish Jethani
Patch Set 18: Change ElemHideBase.fromText to ContentFilter.fromText https://codereview.adblockplus.org/29737558/diff/29816558/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29737558/diff/29816558/lib/filterClasses.js#newcode138 lib/filterClasses.js:138: filter = ...
1 month, 1 week ago (2018-07-06 13:32:16 UTC) #17
Manish Jethani
Patch Set 19: Rebase
1 month, 1 week ago (2018-07-06 13:37:04 UTC) #18
Manish Jethani
Patch Set 20: Fix JSDoc comment
1 month ago (2018-07-10 08:41:20 UTC) #19
kzar
https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js#newcode1019 lib/filterClasses.js:1019: * Script that should be executed This comment seems ...
1 month ago (2018-07-10 12:53:24 UTC) #20
Manish Jethani
https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js#newcode1019 lib/filterClasses.js:1019: * Script that should be executed On 2018/07/10 12:53:22, ...
1 month ago (2018-07-10 18:29:21 UTC) #21
kzar
https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js#newcode1019 lib/filterClasses.js:1019: * Script that should be executed On 2018/07/10 18:29:20, ...
1 month ago (2018-07-10 18:54:01 UTC) #22
Manish Jethani
https://codereview.adblockplus.org/29737558/diff/29826561/lib/snippets.js File lib/snippets.js (right): https://codereview.adblockplus.org/29737558/diff/29826561/lib/snippets.js#newcode64 lib/snippets.js:64: getScriptsForDomain(domain) On 2018/07/10 18:54:00, kzar wrote: > On 2018/07/10 ...
1 month ago (2018-07-11 12:02:10 UTC) #23
kzar
https://codereview.adblockplus.org/29737558/diff/29826561/lib/snippets.js File lib/snippets.js (right): https://codereview.adblockplus.org/29737558/diff/29826561/lib/snippets.js#newcode64 lib/snippets.js:64: getScriptsForDomain(domain) On 2018/07/11 12:02:09, Manish Jethani wrote: > On ...
1 month ago (2018-07-11 12:17:50 UTC) #24
Manish Jethani
https://codereview.adblockplus.org/29737558/diff/29826561/lib/snippets.js File lib/snippets.js (right): https://codereview.adblockplus.org/29737558/diff/29826561/lib/snippets.js#newcode64 lib/snippets.js:64: getScriptsForDomain(domain) On 2018/07/11 12:17:49, kzar wrote: > On 2018/07/11 ...
1 month ago (2018-07-11 12:45:19 UTC) #25
Manish Jethani
Patch Set 21: Rename script to body in ContentFilter Patch Set 22: Improve comment formatting ...
1 month ago (2018-07-11 13:04:28 UTC) #26
Manish Jethani
https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js#newcode1032 lib/filterClasses.js:1032: * rule type, either empty or @ (exception) or ...
1 month ago (2018-07-11 13:16:32 UTC) #27
kzar
https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js#newcode1032 lib/filterClasses.js:1032: * rule type, either empty or @ (exception) or ...
1 month ago (2018-07-11 16:33:14 UTC) #28
Manish Jethani
https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js#newcode1032 lib/filterClasses.js:1032: * rule type, either empty or @ (exception) or ...
1 month ago (2018-07-11 16:42:47 UTC) #29
Sebastian Noack
https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js#newcode1032 lib/filterClasses.js:1032: * rule type, either empty or @ (exception) or ...
1 month ago (2018-07-11 16:47:32 UTC) #30
Manish Jethani
https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js#newcode1032 lib/filterClasses.js:1032: * rule type, either empty or @ (exception) or ...
1 month ago (2018-07-11 16:55:06 UTC) #31
Manish Jethani
https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js#newcode1032 lib/filterClasses.js:1032: * rule type, either empty or @ (exception) or ...
1 month ago (2018-07-11 17:12:11 UTC) #32
kzar
LGTM https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29737558/diff/29826561/lib/filterClasses.js#newcode1032 lib/filterClasses.js:1032: * rule type, either empty or @ (exception) ...
1 month ago (2018-07-11 17:17:54 UTC) #33
Manish Jethani
1 month ago (2018-07-11 17:53:28 UTC) #34
On 2018/07/11 17:17:54, kzar wrote:
> LGTM

Woohoo! Thanks :)
Sign in to reply to this message.

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