Opened 2 years ago

Last modified 2 years ago

#6784 closed change

Update adblockpluscore and adblockplusui dependencies for snippet filters, and the anti-circumvention filter list — at Version 32

Reported by: hfiguiere Assignee: hfiguiere
Priority: P2 Milestone: Adblock-Plus-3.3-for-Chrome-Opera-Firefox
Module: Platform Keywords:
Cc: mjethani, sebastian, kzar, saroyanm, wspee, greiner Blocked By:
Blocking: #6782 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29833597/

Description (last modified by hfiguiere)

Background

We've done some work in the adblockplusui and adblockpluscore repositories towards adding the anti-circumvention filter list, and support for snippet filters. We'd now like to update the dependencies in adblockpluschrome to include that work.

Here are the changes for adblockpluscore:

  • Issue #6690 - Always ignore trailing dot in document domain
  • Issue #6737 - Use String.includes instead of String.indexOf
  • Issue #6559 - Use Map object for known subscriptions, change SpecialSubscription.defaultsMap to Map object
  • Issue #6735 - Store domains in lower case
  • Issue #6619 - Qualify CSS selectors in document style sheet correctly
  • Issue #6727 - Use string rather than map for single-domain filters
  • Issue #6437 - Skip elements not affected by DOM mutations
  • Issue #6689 - Add type property to Subscription class, added anti-CV filter list subscription
  • Issue #6538, #6781 - Add snippets
  • Issue #6733 - Allow empty values in filter options

Here is the change for adblockplusui:

  • Issue #6739 - Adapted code to work with Map implementation of FilterStorage.knownSubscriptions

What to change

Update the adblockpluscore dependency to hg:658f0229baa1 git:b64b8dc, and the adblockplusui dependency to hg:9a652397b9af git:2fe1f42.

Hint for testers

  • Anti-circumvention subscription: on update from a previous release, the "ABP filters" list should be in the list of subscription in the UI. See also #6783.
  • Test that the $rewrite filter option can be used with an empty value (#6733). Note the "=" is still required, so $rewrite= should be accepted, but not $rewrite. FIXME - Give example of rewrite filter with blank value, and explain what it does.
  • Snippets should be enabled, and snippet filters should load. Other changes need to land first as well, so you can't test those yet here. See #6781 and #6782 for the status and testing hints.
  • Internal changes will require a thorough testing of the filter blocking, exceptions, etc.
    • Test domain names with and without trailing dots. See issue #6690
    • Test domain matching with uppercase and lowercase. See issue #6735 FIXME - please give some examples
    • Element hiding emulation filters should be tested. See issue #6619

Change History (32)

comment:1 Changed 2 years ago by hfiguiere

  • Description modified (diff)

comment:2 Changed 2 years ago by hfiguiere

  • Description modified (diff)
  • Summary changed from Upgrade adblockplus core for anti-circumvention and snippets (rev TBD) to Upgrade adblockplus core for anti-circumvention and snippets hg:ba2fb17a1dc1

comment:3 Changed 2 years ago by hfiguiere

  • Description modified (diff)

comment:4 Changed 2 years ago by hfiguiere

We need to update adblockplusui at the same time to get https://issues.adblockplus.org/ticket/6739 that is required.

comment:5 Changed 2 years ago by sebastian

  • Cc saroyanm wspee added

I discussed that with Winsley before. Right after the 3.2 release, #6739 (and only this change) will be merged in the adblockplusui Mercurial repository (they do their development on GitLab and only merge stuff to Mercurial for us to integrate). Then we can update adblockpluscore to the latest revision, and simultaneously update the adblockplusui dependency without pulling in unrelated UI changes at the same time, while not breaking anytinhg (hopefully).

For reference, #6739 (in adblockplusui) and #6559 (in adblockpluscore) have a mutual depdency, i.e. if those changes aren't pulled in simultaneously things will break.

comment:6 Changed 2 years ago by wspee

  • Cc greiner added

comment:7 Changed 2 years ago by hfiguiere

  • Description modified (diff)
  • Summary changed from Upgrade adblockplus core for anti-circumvention and snippets hg:ba2fb17a1dc1 to Upgrade adblockplus core for anti-circumvention and snippets hg:5f851931ffea

comment:8 Changed 2 years ago by hfiguiere

  • Description modified (diff)
  • Summary changed from Upgrade adblockplus core for anti-circumvention and snippets hg:5f851931ffea to Upgrade adblockplus core for anti-circumvention and snippets hg:bec7c35902d3

comment:9 Changed 2 years ago by hfiguiere

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

comment:10 Changed 2 years ago by hfiguiere

  • Owner set to hfiguiere

comment:11 Changed 2 years ago by hfiguiere

  • Description modified (diff)

comment:12 Changed 2 years ago by greiner

See also duplicate ticket #6753.

comment:13 Changed 2 years ago by hfiguiere

I looked for one before even filing this one :-(

comment:14 Changed 2 years ago by hfiguiere

  • Description modified (diff)

comment:15 Changed 2 years ago by hfiguiere

  • Description modified (diff)

comment:16 Changed 2 years ago by hfiguiere

  • Description modified (diff)

comment:17 Changed 2 years ago by mjethani

The last patch for the Snippets feature has landed, but I'm also working on the implementation of a couple of "snippets" and would like to include those as well.

Please wait, I'll update this issue with the final commit hash.

comment:18 Changed 2 years ago by mjethani

  • Description modified (diff)
  • Summary changed from Upgrade adblockplus core for anti-circumvention and snippets hg:bec7c35902d3 to Upgrade adblockplus core for anti-circumvention and snippets hg:8e466288a0c3

comment:19 Changed 2 years ago by mjethani

On second thoughts, we should update the dependency now. I have updated the issue.

comment:20 Changed 2 years ago by kzar

  • Summary changed from Upgrade adblockplus core for anti-circumvention and snippets hg:8e466288a0c3 to Update adblockpluscore and adblockplusui dependencies for snippet filters, and the anti-circumvention filter list

comment:21 Changed 2 years ago by kzar

  • Description modified (diff)

comment:22 Changed 2 years ago by kzar

  • Description modified (diff)

comment:23 Changed 2 years ago by hfiguiere

  • Description modified (diff)

comment:24 Changed 2 years ago by mjethani

  • Blocking 6782 added

comment:25 Changed 2 years ago by greiner

I just noticed that the adblockpluscore dependency update also includes changes for the new anti-circumvention list. Does that mean that we also need to include the UI changes from #6731 already or will those not be necessary yet for this update?

comment:26 Changed 2 years ago by hfiguiere

Ideally we would have the change, but we can do without. The anti-circumvention subscription will be opted-in and you can still disable with the existing UI, and eventually re-add it manually.

comment:27 Changed 2 years ago by hfiguiere

  • Description modified (diff)

comment:28 Changed 2 years ago by kzar

  • Description modified (diff)

comment:29 Changed 2 years ago by kzar

  • Description modified (diff)

comment:30 Changed 2 years ago by mjethani

  • Description modified (diff)

comment:31 Changed 2 years ago by mjethani

@hfiguiere @kzar for the lowercase domain matching tests, we can say that we should test filters with mixed-case domain names (e.g. ExAmPlE.com##.foo) and see that they work. This is the best we can do for manual testing.

For Snippets, I'll add more detailed testing advice in #6781 and #6782.

comment:32 Changed 2 years ago by hfiguiere

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