Opened 16 months ago

Closed 15 months ago

Last modified 14 months ago

#6889 closed change (fixed)

Update adblockpluscore to hg:9ebb3381fcde git:5168283

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

https://codereview.adblockplus.org/29867599/

Description (last modified by sebastian)

Background

We need to update adblockpluscore to hg:9ebb3381fcde git:5168283

This import the follow changes:

Ticket Summary Component
#6871 Extension incorrectly accepts $csp filters with blank value Core
#6855 Remove Subscription object from memory when unsubscribed Core
#6849 Remove redundant logic parsing checksums from filter lists Core
#6848 Add support to hide-if-contains for hiding a different ancestor of the element containing the search string Core
#6847 Add regular expression support to hide-if-contains and hide-if-shadow-contains snippets Core
#6811 Blank values for snippet arguments are ignored Core
#6797 Require active domains in snippet filters Core
#6741 Use ECMAScript 2015 classes Core
#6665 Share functionality and optimizations between ElemHide and ElemHideEmulation Core
#6559 Use maps and sets where appropriate Core
#6504 Implement ElemHideEmulation.useInlineStyles property Core
#6391 Run browser tests with headless Firefox as well as Chromium Core


What to change

  • Change dependencies adblockpluscore to hg:9ebb3381fcde git:5168283
  • also do some changes due to issue #6559 in lib/whitelisting.js,

Integration notes

  • Issue #6855: no change need (see comment:20)
  • Issue #6797: we need to add the filter_snippet_nodomain string to UI (issue #6896)
  • Issue #6849: Remove the noop implementation of Utils.generateChecksum in adblockpluschrome.

Hints for testers

Each of the issues linked above contains updated testing instructions.

Change History (31)

comment:1 Changed 16 months ago by hfiguiere

  • Owner set to hfiguiere

comment:2 Changed 16 months ago by hfiguiere

  • Description modified (diff)
  • Summary changed from Update adblockpluscore to hg:9ebb3381fcde to Update adblockpluscore to hg:9ebb3381fcde git:5168283

comment:3 Changed 16 months ago by mjethani

I'll add some hints for testers tomorrow based on the individual changes that have gone into this.

comment:4 Changed 16 months ago by mjethani

  • Cc mjethani added

comment:5 Changed 15 months ago by mjethani

  • Description modified (diff)

comment:6 Changed 15 months ago by mjethani

  • Cc greiner added

comment:7 Changed 15 months ago by greiner

  • Cc wspee added

comment:8 follow-up: Changed 15 months ago by mjethani

Looking at lib/whitelisting.js, the only change we need to do is replace filter.subscriptions.length with filter.subscriptions.size in two places.

comment:9 follow-up: Changed 15 months ago by sebastian

Any reson to not update right away to the latest revision, i.e. b5fde5993d8b?

comment:10 Changed 15 months ago by sebastian

  • Cc sebastian added

comment:11 Changed 15 months ago by mjethani

@sebastian no reason that I can think of and that'd minimize the changes for #6843.

comment:12 Changed 15 months ago by sebastian

BTW, mind also the integration notes in #6855, #6797 and #6849?

Also #6741, #6665, #6559 and #6504 not being closed is intentional?

comment:13 Changed 15 months ago by mjethani

The last four issues are works in progress, hence still open.

For the first three:

  • #6855: When I looked in UI, I didn't find any reason to change anything; someone from the UI team can take a second look
  • #6797: Good point, we need to add the filter_snippet_nodomain string to UI
  • #6849: We should remove the noop implementation of Utils.generateChecksum in adblockpluschrome

cc: @greiner, @wspee

@hfiguiere can you update the issue with the above?

comment:14 in reply to: ↑ 8 Changed 15 months ago by hfiguiere

Replying to mjethani:

Looking at lib/whitelisting.js, the only change we need to do is replace filter.subscriptions.length with filter.subscriptions.size in two places.

I have them here locally already.

comment:15 in reply to: ↑ 9 Changed 15 months ago by hfiguiere

Replying to sebastian:

Any reson to not update right away to the latest revision, i.e. b5fde5993d8b?

Because we change the API (function renamed) specifically for issue #6843 so I thought it was natural to do it at the same time (in issue #6843)

comment:16 Changed 15 months ago by sebastian

So you say if you update beyonf 9ebb3381fcde things will break without further adaptions, that will happen in #6843 anyway? That makes sense I guess.

comment:17 Changed 15 months ago by hfiguiere

  • Description modified (diff)

comment:18 Changed 15 months ago by hfiguiere

  • Description modified (diff)

comment:19 Changed 15 months ago by greiner

  • Description modified (diff)

Added reference to related UI ticket.

comment:20 Changed 15 months ago by greiner

Since #6559 also affects adblockplusui, you can find the corresponding dependency update ticket for that at #6892.

Issue #6855 doesn't seem to be any reason to change anything; someone from the UI team can take a second look

I can confirm that this doesn't seem to have an impact on adblockplusui because the only way we interact with FilterStorage.knownSubscriptions is for checking whether a filter list is installed.

comment:21 Changed 15 months ago by hfiguiere

  • Description modified (diff)

comment:22 Changed 15 months ago by hfiguiere

  • Description modified (diff)

comment:23 Changed 15 months ago by hfiguiere

  • Blocking 6897 added

comment:24 Changed 15 months ago by hfiguiere

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

comment:25 Changed 15 months ago by sebastian

  • Description modified (diff)
  • Priority changed from Unknown to P3
  • Ready set

comment:27 Changed 15 months ago by hfiguiere

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:28 Changed 15 months ago by hfiguiere

  • Blocking 6843 removed

comment:29 Changed 15 months ago by hfiguiere

  • Blocking 6843 added

comment:30 Changed 15 months ago by hfiguiere

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next

comment:31 Changed 14 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. All related tickets have been tested succesfully.

ABP 3.3.2.2175
Firefox 62 / 51 / Windows 10
Chrome 69 / 49 / Windows 10
Opera 56 / 36 / Windows 10

Note: See TracTickets for help on using tickets.