Opened on 08/27/2018 at 08:49:02 PM
Closed on 08/28/2018 at 05:39:26 PM
Last modified on 10/24/2018 at 08:01:13 AM
#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): |
Description (last modified by sebastian)
Background
We need to update adblockpluscore to hg:9ebb3381fcde git:5168283
This import the follow changes:
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.
Attachments (0)
Change History (31)
comment:1 Changed on 08/27/2018 at 09:14:34 PM by hfiguiere
- Owner set to hfiguiere
comment:2 Changed on 08/27/2018 at 09:20:12 PM by hfiguiere
- Description modified (diff)
- Summary changed from Update adblockpluscore to hg:9ebb3381fcde to Update adblockpluscore to hg:9ebb3381fcde git:5168283
comment:3 Changed on 08/27/2018 at 10:07:19 PM by mjethani
comment:4 Changed on 08/27/2018 at 10:07:38 PM by mjethani
- Cc mjethani added
comment:6 Changed on 08/28/2018 at 09:17:32 AM by mjethani
- Cc greiner added
comment:7 Changed on 08/28/2018 at 10:14:18 AM by greiner
- Cc wspee added
comment:8 follow-up: ↓ 14 Changed on 08/28/2018 at 10:33:19 AM 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: ↓ 15 Changed on 08/28/2018 at 10:45:37 AM by sebastian
Any reson to not update right away to the latest revision, i.e. b5fde5993d8b?
comment:10 Changed on 08/28/2018 at 10:45:55 AM by sebastian
- Cc sebastian added
comment:11 Changed on 08/28/2018 at 10:55:00 AM by mjethani
@sebastian no reason that I can think of and that'd minimize the changes for #6843.
comment:12 Changed on 08/28/2018 at 11:03:31 AM by sebastian
comment:13 Changed on 08/28/2018 at 11:17:18 AM 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 on 08/28/2018 at 12:08:01 PM 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 on 08/28/2018 at 12:10:27 PM by hfiguiere
comment:16 Changed on 08/28/2018 at 12:29:26 PM 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 on 08/28/2018 at 01:39:25 PM by hfiguiere
- Description modified (diff)
comment:18 Changed on 08/28/2018 at 01:39:59 PM by hfiguiere
- Description modified (diff)
comment:19 Changed on 08/28/2018 at 02:04:36 PM by greiner
- Description modified (diff)
Added reference to related UI ticket.
comment:20 Changed on 08/28/2018 at 02:17:01 PM 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 on 08/28/2018 at 02:41:13 PM by hfiguiere
- Description modified (diff)
comment:22 Changed on 08/28/2018 at 02:50:51 PM by hfiguiere
- Description modified (diff)
comment:23 Changed on 08/28/2018 at 02:51:01 PM by hfiguiere
- Blocking 6897 added
comment:24 Changed on 08/28/2018 at 02:55:31 PM by hfiguiere
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:25 Changed on 08/28/2018 at 03:53:11 PM by sebastian
- Description modified (diff)
- Priority changed from Unknown to P3
- Ready set
comment:26 Changed on 08/28/2018 at 05:38:53 PM by abpbot
A commit referencing this issue has landed:
Issue 6889 - Upgrade adblockpluscore to 9ebb3381fcde and integrate the changes.
comment:27 Changed on 08/28/2018 at 05:39:26 PM by hfiguiere
- Resolution set to fixed
- Status changed from reviewing to closed
comment:28 Changed on 08/28/2018 at 05:42:42 PM by hfiguiere
- Blocking 6843 removed
comment:29 Changed on 08/28/2018 at 05:44:14 PM by hfiguiere
- Blocking 6843 added
comment:30 Changed on 08/28/2018 at 07:44:41 PM by hfiguiere
- Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
comment:31 Changed on 10/24/2018 at 08:01:13 AM 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
I'll add some hints for testers tomorrow based on the individual changes that have gone into this.