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

Issue 29760707: Issue 6622 - Implement $rewrite filter option (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 weeks, 5 days ago by hub
Modified:
2 days, 14 hours ago
Base URL:
https://hg.adblockplus.org/adblockpluschrome/
Visibility:
Public.

Description

Issue 6622 - Implement $rewrite filter option

Patch Set 1 #

Total comments: 3

Patch Set 2 : Filter rewrite is in core. Some cleanup #

Patch Set 3 : Refactor the notification code. #

Total comments: 16

Patch Set 4 : Simplified the redirect logic. #

Total comments: 2

Patch Set 5 : Remove uneeded check #

Total comments: 5

Patch Set 6 : No longer have a "REWRITE" type in devtools. Log the rewritten to URL. #

Total comments: 1

Patch Set 7 : Fix a linting error. #

Total comments: 10

Patch Set 8 : Log all request, even failed rewrites. + comment #

Total comments: 9

Patch Set 9 : Some comments adjustments. #

Patch Set 10 : Rebased #

Total comments: 2

Patch Set 11 : rename rewritten to rewrittenTo for consistency? #

Total comments: 2

Patch Set 12 : comment fixes #

Total comments: 6

Patch Set 13 : Rename rewrittenTo to rewrittenUrl. Adjust some comments. #

Total comments: 4

Patch Set 14 : Updated proper revision for adblockpluscore. No dependency change for ui #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -4 lines) Patch
M dependencies View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -1 line 0 comments Download
M lib/requestBlocker.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +22 lines, -3 lines 0 comments Download

Messages

Total messages: 43
hub
3 weeks, 5 days ago (2018-04-24 20:33:39 UTC) #1
hub
3 weeks, 5 days ago (2018-04-24 20:34:00 UTC) #2
hub
https://codereview.adblockplus.org/29760707/diff/29760708/dependencies File dependencies (right): https://codereview.adblockplus.org/29760707/diff/29760708/dependencies#newcode4 dependencies:4: adblockpluscore = adblockpluscore hg:10e78c931816 git:8508af0 This need to be ...
3 weeks, 5 days ago (2018-04-24 20:34:41 UTC) #3
hub
https://codereview.adblockplus.org/29760707/diff/29760708/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29760708/lib/requestBlocker.js#newcode203 lib/requestBlocker.js:203: let rewritten = filter.rewrite.replace("%1", matches[0]); I actually wonder if ...
3 weeks, 5 days ago (2018-04-24 20:39:06 UTC) #4
hub
The substitution pattern is different now. It is $1 (but this is only visible in ...
3 weeks, 5 days ago (2018-04-25 03:39:09 UTC) #5
Manish Jethani
https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js#newcode204 lib/requestBlocker.js:204: if (filter instanceof BlockingFilter && filter.rewrite) I think maybe ...
2 weeks, 6 days ago (2018-04-30 20:24:59 UTC) #6
hub
https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js#newcode204 lib/requestBlocker.js:204: if (filter instanceof BlockingFilter && filter.rewrite) On 2018/04/30 20:24:58, ...
2 weeks, 5 days ago (2018-05-01 18:32:37 UTC) #7
Sebastian Noack
https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js#newcode73 lib/requestBlocker.js:73: // POPUP, CSP, REWRITE and ELEMHIDE filters aren't mapped ...
2 weeks, 4 days ago (2018-05-02 15:04:59 UTC) #8
Manish Jethani
https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js#newcode204 lib/requestBlocker.js:204: if (filter instanceof BlockingFilter && filter.rewrite) On 2018/05/02 15:04:59, ...
2 weeks, 4 days ago (2018-05-02 16:13:45 UTC) #9
Sebastian Noack
https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js#newcode204 lib/requestBlocker.js:204: if (filter instanceof BlockingFilter && filter.rewrite) On 2018/05/02 16:13:45, ...
2 weeks, 4 days ago (2018-05-02 16:40:09 UTC) #10
Manish Jethani
https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js#newcode204 lib/requestBlocker.js:204: if (filter instanceof BlockingFilter && filter.rewrite) On 2018/05/02 16:40:09, ...
2 weeks, 4 days ago (2018-05-02 17:45:50 UTC) #11
hub
https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js#newcode73 lib/requestBlocker.js:73: // POPUP, CSP, REWRITE and ELEMHIDE filters aren't mapped ...
2 weeks, 4 days ago (2018-05-03 01:18:03 UTC) #12
Manish Jethani
https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js#newcode204 lib/requestBlocker.js:204: if (filter instanceof BlockingFilter && filter.rewrite) On 2018/05/03 01:18:02, ...
2 weeks, 4 days ago (2018-05-03 02:51:17 UTC) #13
Sebastian Noack
https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js#newcode204 lib/requestBlocker.js:204: if (filter instanceof BlockingFilter && filter.rewrite) On 2018/05/02 17:45:50, ...
2 weeks, 3 days ago (2018-05-03 15:55:47 UTC) #14
hub
Updated patch https://codereview.adblockplus.org/29760707/diff/29768693/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29768693/lib/requestBlocker.js#newcode205 lib/requestBlocker.js:205: if (!rewritten || rewritten == urlString) On ...
2 weeks, 3 days ago (2018-05-03 18:43:48 UTC) #15
Manish Jethani
https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js#newcode204 lib/requestBlocker.js:204: if (filter instanceof BlockingFilter && filter.rewrite) On 2018/05/03 15:55:47, ...
2 weeks, 2 days ago (2018-05-04 10:02:00 UTC) #16
Sebastian Noack
https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js#newcode204 lib/requestBlocker.js:204: if (filter instanceof BlockingFilter && filter.rewrite) On 2018/05/04 10:01:59, ...
2 weeks, 2 days ago (2018-05-04 15:17:17 UTC) #17
Manish Jethani
https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29762749/lib/requestBlocker.js#newcode204 lib/requestBlocker.js:204: if (filter instanceof BlockingFilter && filter.rewrite) On 2018/05/04 15:17:16, ...
2 weeks, 2 days ago (2018-05-04 15:37:48 UTC) #18
Sebastian Noack
https://codereview.adblockplus.org/29760707/diff/29769599/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29769599/lib/requestBlocker.js#newcode209 lib/requestBlocker.js:209: type = "REWRITE"; On 2018/05/04 15:37:47, Manish Jethani wrote: ...
2 weeks, 2 days ago (2018-05-04 15:46:49 UTC) #19
hub
Updated patch. https://codereview.adblockplus.org/29760707/diff/29769599/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29769599/lib/requestBlocker.js#newcode205 lib/requestBlocker.js:205: if (rewritten == urlString) On 2018/05/04 15:37:47, ...
2 weeks, 2 days ago (2018-05-04 18:27:58 UTC) #20
hub
UI code changes are at https://codereview.adblockplus.org/29770586/
2 weeks, 2 days ago (2018-05-04 18:30:15 UTC) #21
Sebastian Noack
https://codereview.adblockplus.org/29760707/diff/29770582/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29770582/lib/requestBlocker.js#newcode206 lib/requestBlocker.js:206: // we couldn't do the rewrite, so just let ...
2 weeks, 2 days ago (2018-05-04 19:46:37 UTC) #22
Manish Jethani
https://codereview.adblockplus.org/29760707/diff/29770582/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29770582/lib/requestBlocker.js#newcode206 lib/requestBlocker.js:206: // we couldn't do the rewrite, so just let ...
2 weeks, 2 days ago (2018-05-04 20:06:12 UTC) #23
Manish Jethani
https://codereview.adblockplus.org/29760707/diff/29770582/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29770582/lib/requestBlocker.js#newcode206 lib/requestBlocker.js:206: // we couldn't do the rewrite, so just let ...
2 weeks, 2 days ago (2018-05-04 20:27:24 UTC) #24
hub
https://codereview.adblockplus.org/29760707/diff/29770582/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29770582/lib/requestBlocker.js#newcode206 lib/requestBlocker.js:206: // we couldn't do the rewrite, so just let ...
2 weeks, 2 days ago (2018-05-04 20:48:08 UTC) #25
Manish Jethani
https://codereview.adblockplus.org/29760707/diff/29770582/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29770582/lib/requestBlocker.js#newcode204 lib/requestBlocker.js:204: let rewritten = filter.rewriteUrl(urlString); We don't need rewritten, we ...
2 weeks, 2 days ago (2018-05-04 20:49:47 UTC) #26
hub
https://codereview.adblockplus.org/29760707/diff/29770582/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29770582/lib/requestBlocker.js#newcode204 lib/requestBlocker.js:204: let rewritten = filter.rewriteUrl(urlString); On 2018/05/04 20:49:47, Manish Jethani ...
2 weeks, 2 days ago (2018-05-04 20:51:50 UTC) #27
Manish Jethani
https://codereview.adblockplus.org/29760707/diff/29770597/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29770597/lib/requestBlocker.js#newcode217 lib/requestBlocker.js:217: thirdParty, sitekey, specificOnly, filter, rewritten); For what it's worth, ...
2 weeks, 2 days ago (2018-05-05 00:54:12 UTC) #28
Sebastian Noack
https://codereview.adblockplus.org/29760707/diff/29770597/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29770597/lib/requestBlocker.js#newcode198 lib/requestBlocker.js:198: let rewritten; Nit: Since these variables are no always ...
2 weeks, 1 day ago (2018-05-05 13:46:54 UTC) #29
hub
Updated patch. https://codereview.adblockplus.org/29760707/diff/29770597/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29770597/lib/requestBlocker.js#newcode198 lib/requestBlocker.js:198: let rewritten; On 2018/05/05 13:46:53, Sebastian Noack ...
1 week, 6 days ago (2018-05-07 20:13:04 UTC) #30
hub
This patch needs to be rebased now
1 week, 3 days ago (2018-05-10 13:52:55 UTC) #31
hub
Rebased patch.
1 week, 3 days ago (2018-05-10 14:35:25 UTC) #32
Sebastian Noack
https://codereview.adblockplus.org/29760707/diff/29777643/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29777643/lib/requestBlocker.js#newcode211 lib/requestBlocker.js:211: sitekey, specificOnly, rewrittenTo: rewritten Nit: How about just calling ...
1 week, 3 days ago (2018-05-10 15:09:52 UTC) #33
hub
updated patch https://codereview.adblockplus.org/29760707/diff/29777643/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29777643/lib/requestBlocker.js#newcode211 lib/requestBlocker.js:211: sitekey, specificOnly, rewrittenTo: rewritten On 2018/05/10 15:09:52, ...
1 week, 3 days ago (2018-05-10 16:08:48 UTC) #34
Manish Jethani
This look good to me modulo the dependency update. https://codereview.adblockplus.org/29760707/diff/29777649/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29777649/lib/requestBlocker.js#newcode196 lib/requestBlocker.js:196: ...
1 week, 2 days ago (2018-05-11 14:11:19 UTC) #35
hub
Patch updated https://codereview.adblockplus.org/29760707/diff/29777649/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29777649/lib/requestBlocker.js#newcode196 lib/requestBlocker.js:196: // If no rewrite happened (error, diff ...
1 week, 2 days ago (2018-05-11 15:58:26 UTC) #36
kzar
https://codereview.adblockplus.org/29760707/diff/29778594/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29778594/lib/requestBlocker.js#newcode73 lib/requestBlocker.js:73: // These filter types aren't mapped to resource types. ...
5 days, 16 hours ago (2018-05-15 14:30:30 UTC) #37
Manish Jethani
https://codereview.adblockplus.org/29760707/diff/29778594/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29778594/lib/requestBlocker.js#newcode189 lib/requestBlocker.js:189: let rewrittenTo; On 2018/05/15 14:30:29, kzar wrote: > Nit: ...
5 days, 16 hours ago (2018-05-15 14:44:25 UTC) #38
Sebastian Noack
https://codereview.adblockplus.org/29760707/diff/29778594/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29778594/lib/requestBlocker.js#newcode189 lib/requestBlocker.js:189: let rewrittenTo; On 2018/05/15 14:30:29, kzar wrote: > Nit: ...
5 days, 16 hours ago (2018-05-15 14:44:36 UTC) #39
hub
Updated patch. UI patch updated as well to match. https://codereview.adblockplus.org/29760707/diff/29778594/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29760707/diff/29778594/lib/requestBlocker.js#newcode73 lib/requestBlocker.js:73: ...
5 days, 14 hours ago (2018-05-15 16:31:27 UTC) #40
Sebastian Noack
Can you please save your devbuild announcement draft in Textpattern, so that we can publish ...
2 days, 15 hours ago (2018-05-18 15:13:31 UTC) #41
hub
patch updated with the proper revision https://codereview.adblockplus.org/29760707/diff/29782620/dependencies File dependencies (right): https://codereview.adblockplus.org/29760707/diff/29782620/dependencies#newcode4 dependencies:4: adblockpluscore = adblockpluscore ...
2 days, 15 hours ago (2018-05-18 15:28:49 UTC) #42
Sebastian Noack
2 days, 15 hours ago (2018-05-18 15:40:08 UTC) #43
LGTM. But don't push before the devbuild announcement is ready to be published
in Textpattern.
Sign in to reply to this message.

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