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

Issue 29737558: Issue 6538 - Implement support for snippet filters

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 2 weeks ago by Manish Jethani
Modified:
6 days, 23 hours ago
Reviewers:
kzar, sergei, hub
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 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+291 lines, -24 lines) Patch
M lib/filterClasses.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 5 chunks +62 lines, -19 lines 0 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 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: 10
Manish Jethani
2 months, 2 weeks ago (2018-03-30 20:19:00 UTC) #1
kzar
(Adding Mapx to Cc at his request.)
2 months, 2 weeks ago (2018-04-03 08:59:47 UTC) #2
Manish Jethani
Patch Set 2: Check for null and blank string instead
2 months, 2 weeks ago (2018-04-03 10:46:30 UTC) #3
sergei
Could you please add tests?
2 months, 1 week 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 ...
1 month, 3 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 ...
1 month, 3 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, ...
1 month, 2 weeks 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 ...
1 month, 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 ...
3 weeks, 5 days ago (2018-05-23 03:59:48 UTC) #9
Manish Jethani
6 days, 23 hours ago (2018-06-11 10:07:50 UTC) #10
Patch Set 14: Rebase

ElemHideBase.selectorDomains is gone, as has ScriptFilter.scriptDomains now.

Still not sure about exceptions, but I think eventually ElemHideException should
be renamed to ScriptException, extending ScriptFilter or a separate
ScriptFilterBase if you will, and it should apply to "##", "#?#", and "#$#"
filters. Also see Trac #6726.
Sign in to reply to this message.

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