|
|
Created:
May 1, 2018, 7:56 p.m. by hub Modified:
June 12, 2018, 12:44 p.m. CC:
kzar Base URL:
https://hg.adblockplus.org/web.adblockplus.org/ Visibility:
Public. |
DescriptionIssue 6626 - Document the rewrite filter option
Patch Set 1 #
Total comments: 3
Patch Set 2 : Update from gdoc #
Total comments: 3
Patch Set 3 : Fixes #
Total comments: 2
Patch Set 4 : Addressed Julian's comments. Added a note about the restriction on content for rewriting. #MessagesTotal messages: 14
The feature hasn't landed yet, and still subject to change. Stashing this in for now
Also note that we need a development build announcement. https://codereview.adblockplus.org/29767583/diff/29767584/pages/filters.html File pages/filters.html (right): https://codereview.adblockplus.org/29767583/diff/29767584/pages/filters.html#... pages/filters.html:185: {{rewriteoption Rewrite the URL with the option <code><fix>rewrite=</fix></code>. You must use a regular expression filter, using <code><fix>$n</fix></code> in the rewrite argument, with n an number between 1 and 100, will insert the n-th parenthesis submatch of the filter. Anything not explicitely matched by the regular expression will be added in the rewritten string. For example <code><fix>/(server.com\/assets\/file.php)\?.*$/$rewrite=$1</fix></code> will rewrite the URL by removing everything that comes after ".php". If there is no query parameter (i.e. no '?'), this filter won't match. The rewrite parameter us the same syntax as JavaScript <code><fix>String.prototype.replace()</fix></code> when using a regular expression instead of a substring.}} I rather review larger texts with Google Docs: https://docs.google.com/document/d/1OoXfJ3ZHpXxv5VXvIoovwefEMOXmRBYd-yYdPAJ2f...
https://codereview.adblockplus.org/29767583/diff/29767584/pages/filters.html File pages/filters.html (right): https://codereview.adblockplus.org/29767583/diff/29767584/pages/filters.html#... pages/filters.html:185: {{rewriteoption Rewrite the URL with the option <code><fix>rewrite=</fix></code>. You must use a regular expression filter, using <code><fix>$n</fix></code> in the rewrite argument, with n an number between 1 and 100, will insert the n-th parenthesis submatch of the filter. Anything not explicitely matched by the regular expression will be added in the rewritten string. For example <code><fix>/(server.com\/assets\/file.php)\?.*$/$rewrite=$1</fix></code> will rewrite the URL by removing everything that comes after ".php". If there is no query parameter (i.e. no '?'), this filter won't match. The rewrite parameter us the same syntax as JavaScript <code><fix>String.prototype.replace()</fix></code> when using a regular expression instead of a substring.}} On 2018/05/05 14:48:37, Sebastian Noack wrote: > I rather review larger texts with Google Docs: > https://docs.google.com/document/d/1OoXfJ3ZHpXxv5VXvIoovwefEMOXmRBYd-yYdPAJ2f... The edited version in the Google Doc looks good to me. But Manish probably should have a look too.
Patch updated from gdoc https://codereview.adblockplus.org/29767583/diff/29767584/pages/filters.html File pages/filters.html (right): https://codereview.adblockplus.org/29767583/diff/29767584/pages/filters.html#... pages/filters.html:185: {{rewriteoption Rewrite the URL with the option <code><fix>rewrite=</fix></code>. You must use a regular expression filter, using <code><fix>$n</fix></code> in the rewrite argument, with n an number between 1 and 100, will insert the n-th parenthesis submatch of the filter. Anything not explicitely matched by the regular expression will be added in the rewritten string. For example <code><fix>/(server.com\/assets\/file.php)\?.*$/$rewrite=$1</fix></code> will rewrite the URL by removing everything that comes after ".php". If there is no query parameter (i.e. no '?'), this filter won't match. The rewrite parameter us the same syntax as JavaScript <code><fix>String.prototype.replace()</fix></code> when using a regular expression instead of a substring.}} On 2018/05/05 14:48:37, Sebastian Noack wrote: > I rather review larger texts with Google Docs: > https://docs.google.com/document/d/1OoXfJ3ZHpXxv5VXvIoovwefEMOXmRBYd-yYdPAJ2f... Done.
LGTM (for the content), but again Manish should have a look as well in case he didn't yet.
Also some comments in the Google Doc. https://codereview.adblockplus.org/29767583/diff/29774555/pages/filter-cheats... File pages/filter-cheatsheet.html (right): https://codereview.adblockplus.org/29767583/diff/29774555/pages/filter-cheats... pages/filter-cheatsheet.html:629: <td>{{rewritedesc Specify a rewrite rule for the URL to be performed before downloading. The filter should be a regular expression. Use <code>$n</code> to insert submatches into the rewritten URL. See JavaScript own <code>String.prototype.replace()</code>.}}</td> The filter can very well be a string. e.g. /foo^$rewrite=/bar I think we should write this as "If the filter is a regular expression, use $n to insert submatches ....". https://codereview.adblockplus.org/29767583/diff/29774555/pages/filters.html File pages/filters.html (right): https://codereview.adblockplus.org/29767583/diff/29774555/pages/filters.html#... pages/filters.html:185: {{rewriteoption Rewrite the URL with the option <code><fix>rewrite=</fix></code>. You may want to create a regular expression filter to perform the rewrite operation. In that case, you can use <code><fix>$n</fix></code> in the rewrite argument, with n being a number between 1 and 100, to insert the n-th parenthesis submatch of the filter regular expression. Anything not explicitly matched by it will be added in the rewritten string. For example <code><fix>/(server.com\/assets\/file.php)\?.*$/$rewrite=$1</fix></code> will strip everything that comes after ".php" and redirects the request to the resulting URL. If there is no query parameter (i.e. no '?'), this filter won't match. The rewrite parameter has the same syntax as JavaScript’s <code><fix>String.prototype.replace()</fix></code> when using a regular expression instead of a substring. If both, a filter with/without <code><fix>$rewrite</fix></code> option matches, the behavior is undefined, i.e. the request might either be blocked or redirected. (Adblock Plus 3.2 or higher required.)}} We should probably also mention about the requirement that the rewritten URL should also be of the same origin. If we're supporting relative URLs (we are as I see it), we can mention that here as well, but then I also wonder if that's an unnecessary detail that can be skipped in the documentation.
https://codereview.adblockplus.org/29767583/diff/29774555/pages/filter-cheats... File pages/filter-cheatsheet.html (right): https://codereview.adblockplus.org/29767583/diff/29774555/pages/filter-cheats... pages/filter-cheatsheet.html:629: <td>{{rewritedesc Specify a rewrite rule for the URL to be performed before downloading. The filter should be a regular expression. Use <code>$n</code> to insert submatches into the rewritten URL. See JavaScript own <code>String.prototype.replace()</code>.}}</td> On 2018/05/10 02:47:39, Manish Jethani wrote: > The filter can very well be a string. e.g. /foo^$rewrite=/bar > > I think we should write this as "If the filter is a regular expression, use $n > to insert submatches ....". Done.
Thanks hub (and sorry about the delay if you were waiting for me). Only one issue below. https://codereview.adblockplus.org/29767583/diff/29782627/pages/filter-cheats... File pages/filter-cheatsheet.html (right): https://codereview.adblockplus.org/29767583/diff/29782627/pages/filter-cheats... pages/filter-cheatsheet.html:629: <td>{{rewritedesc Specify a rewrite rule for the URL to be performed before downloading. If the filter is a regular expression, use <code>$n</code> to insert submatches into the rewritten URL. See JavaScript own <code>String.prototype.replace()</code>.}}</td> Missing fix tags inside code tags.
updated patch. https://codereview.adblockplus.org/29767583/diff/29782627/pages/filter-cheats... File pages/filter-cheatsheet.html (right): https://codereview.adblockplus.org/29767583/diff/29782627/pages/filter-cheats... pages/filter-cheatsheet.html:629: <td>{{rewritedesc Specify a rewrite rule for the URL to be performed before downloading. If the filter is a regular expression, use <code>$n</code> to insert submatches into the rewritten URL. See JavaScript own <code>String.prototype.replace()</code>.}}</td> On 2018/05/22 13:30:41, juliandoucette wrote: > Missing fix tags inside code tags. Done.
LGTM
Hubert, can we close this issue now?
We will push these changes when we release Adblock Plus 3.2. |