Opened 18 months ago

Last modified 16 months ago

#6622 closed change

Implement rewrite filter option — at Version 10

Reported by: hfiguiere Assignee: hfiguiere
Priority: P2 Milestone: Adblock-Plus-3.2-for-Chrome-Opera-Firefox
Module: Platform Keywords: circumvention
Cc: sebastian, kzar, Ross, mapx, arthur Blocked By: #6592
Blocking: #6242 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29760707/

Description (last modified by hfiguiere)

Background

Issue #6592 introduce a new rewrite option to rewrite URL instead of blocking. This is to implement the WebExt side of the rewrite.

Rewriting a URL is, instead of blocking the request, modifying the request URL to load that instead.

What to change

  • Update dependencies to have the patch for issue #6592 from core.
    • Issue #6592 - Implement "rewrite" filter option
    • Issue #6649 - Allow u flag in :-abp-contains()
    • Noissue - Update JSDoc @type for Matcher.filterByKeyword
    • Noissue - Use plural for element hiding domains
    • Issue #6562 - Remove filter keys from the element hiding code
    • Noissue - Compare individual characters instead of substring
    • Issue #6610 - Prefer inline style for :-abp-properties()
    • Issue #6437 - Filter out patterns that do not match DOM mutations
    • Issue #6562 - Remove the unused getSelectors function
    • Issue #6562 - Remove the unused provideFilterKeys parameter
  • Implement the rewrite in requestBlocker.js by calling the function for core that will perform it. This is handled as an exception of blocking in the same handler.
  • Update UI to display the "rewritten to" information in the devtools.
  • This will require a dependency update on adblockplusui.
    • TBD

Hint for testers

  • Test that blocking filters without rewrite aren't broken
  • Ensure that the rewrites are logged accordingly into the developer tools.
  • Test filters with rewrite option.
    • Basic rewrite: add filter /(testpages\.adblockplus\.org\/css\/testpages\.css)\?14$/$rewrite=$1?42 and visit https://testpages.adblockplus.org/en/testcases/css/03 . You should notice the rewrite in the Adblock Plus developer tool panel.
    • Rewrite to relative URL. Should succeed. Add filter /testpages\.adblockplus\.org(\/css\/testpages\.css)\?14$/$rewrite=$1?42 (and make sure the previous filter is removed). You should notice the rewrite in the Adblock Plus developer tool panel.
    • Rewrite to different origin: Should fail and the original request let through. Add filter /testpages\.adblockplus\.org(\/css\/testpages\.css)\?14$/$rewrite=bogus.adblockplus.org$1?42 (and make sure the previous filter is removed). You should notice the rewrite in the Adblock Plus developer tool panel, showing that the rewritten URL is the same as the original one.

Change History (10)

comment:1 Changed 18 months ago by hfiguiere

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:2 Changed 18 months ago by hfiguiere

  • Keywords circumvention added

comment:3 Changed 18 months ago by hfiguiere

  • Blocking 6242 added

comment:4 Changed 18 months ago by hfiguiere

  • Description modified (diff)

comment:5 Changed 18 months ago by hfiguiere

  • Description modified (diff)
  • Review URL(s) modified (diff)

comment:6 Changed 18 months ago by greiner

For reference: I've created gitlab#76 to come up with a simple design based on the one provided via the review.

While that's being worked on, we can continue the review with the existing code.

comment:7 Changed 18 months ago by hfiguiere

  • Description modified (diff)

comment:8 Changed 18 months ago by hfiguiere

  • Description modified (diff)

comment:9 Changed 18 months ago by kzar

Please can you expand on the introduction to explain what the idea of the $rewrite option is and to include an example filter with an explanation of what it does?

Please can you improve the "Hints for testers" section? For example you say "Test filters with rewrite option" without giving any examples or saying how they are expected to work.

comment:10 Changed 18 months ago by hfiguiere

  • Description modified (diff)
Note: See TracTickets for help on using tickets.