Opened on 03/21/2017 at 02:27:55 PM

Closed on 03/22/2017 at 09:31:36 AM

Last modified on 06/28/2017 at 01:33:09 PM

#5023 closed change (fixed)

Update adblockplusui dependency to 8ceaabb9c639

Reported by:	jsonesen	Assignee:	jsonesen
Priority:	P3	Milestone:	Adblock-Plus-1.13.3-for-Chrome-Opera
Module:	Platform	Keywords:
Cc:	sebastian, kzar	Blocked By:
Blocking:		Platform:	Chrome
Ready:	yes	Confidential:	no
Tester:	Ross	Verified working:	yes
Review URL(s):	https://codereview.adblockplus.org/29391717/

Description (last modified by sebastian)

Background

This dependency update pulls in following changes:

Ticket	Summary	Owner	Type	Status	Priority	Milestone
#4871	Start using ESLint for adblockplusui repository	kzar	change	closed	P3
#4915	Expose ext.i18n for background pages	wspee	change	closed	P2	Adblock-Plus-2.9-for-Firefox
#4968	SyntaxError in firefox after adblockplusui 7be8b3f3c5fb	trev	defect	closed	P1
#5019	Anti-adblock notification not added on first-run	jsonesen	defect	closed	P2

The only change intended to be visible to users is a fix for a bug that caused the "Hide targeted messages?" being added/removed incorrectly, not showing up before the browser is restarted after installation (#5019).

While the other changes are under the hood, they and in particular #4871, have a quite large impact, requiring thorough testing of the options page, first run page and all other UI that add filters or subscriptions.

What to change

Update the adblockplusui dependency to 8ceaabb9c639.
Adapt for an API change from #4871.
- i18n_timeDateStrings => i18nTimeDateStrings

Change History (12)

comment:1 Changed on 03/21/2017 at 02:28:55 PM by jsonesen

Cc snoack kzar added

comment:2 Changed on 03/21/2017 at 04:11:08 PM by jsonesen

Cc sebastian added; snoack removed

comment:3 Changed on 03/21/2017 at 08:59:32 PM by sebastian

Component changed from Unknown to Platform
Description modified (diff)
Priority changed from Unknown to P3
Ready set
Summary changed from Update dependencies in adblockpluschrome for adblockplusui to Update adblockplusui dependency to 8ceaabb9c639

comment:4 Changed on 03/21/2017 at 09:14:28 PM by sebastian

Thanks for writing this issue. Some advice for the future:

The module for code (except UI) in adblockpluschrome is Platform, as documented here.
Try to describe changes, in issues, on a high level, avoiding unnecessary implementation details.
Give issues a distinctive summary. "Update dependencies in X for Y" is not distinct because it unlikely will be the only issue about updating Y in X. Give it some context, if nothing else the dependency's revision will do.
For dependency updates, always list all changes that are pulled in (but ignore changes to files that aren't included in the resulting builds). Outline the changes supposed to be user visible. And give testers an idea what else needs to be tested, in particular for under the hood changes.

Last edited on 03/21/2017 at 09:18:43 PM by sebastian

comment:5 Changed on 03/21/2017 at 09:21:51 PM by sebastian

Description modified (diff)

comment:6 Changed on 03/22/2017 at 07:59:05 AM by jsonesen

Thanks for the tips. They all make perfect sense. In the future I will consider these when writing tickets.

comment:7 Changed on 03/22/2017 at 08:42:46 AM by sebastian

Review URL(s) modified (diff)

comment:8 Changed on 03/22/2017 at 08:42:57 AM by sebastian

Status changed from new to reviewing

comment:9 Changed on 03/22/2017 at 09:30:43 AM by abpbot

A commit referencing this issue has landed:
Issue 5023 - Update adblockplusui dependency to 8ceaabb9c639

comment:10 Changed on 03/22/2017 at 09:31:36 AM by jsonesen

Resolution set to fixed
Status changed from reviewing to closed

comment:11 Changed on 03/22/2017 at 09:36:04 AM by sebastian

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

When closing an issue, after landing changes in a repository that does releases, please also set the Milestone to the corresponding release.

Last edited on 03/22/2017 at 09:36:16 AM by sebastian

comment:12 Changed on 06/28/2017 at 01:33:09 PM by Ross

Tester changed from Unknown to Ross
Verified working set

Anti-adblock notification works as expected in Chrome. Does not currently display in Opera (#5354).

Chrome 49 / 59 / Windows 7
Opera 36 / 45 / Windows 7

Context Navigation