Opened 5 months ago

Closed 5 months ago

Last modified 8 weeks ago

#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:


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 5 months ago by jsonesen

  • Cc snoack kzar added

comment:2 Changed 5 months ago by jsonesen

  • Cc sebastian added; snoack removed

comment:3 Changed 5 months ago 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 5 months ago 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 5 months ago by sebastian (previous) (diff)

comment:5 Changed 5 months ago by sebastian

  • Description modified (diff)

comment:6 Changed 5 months ago by jsonesen

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

comment:7 Changed 5 months ago by sebastian

  • Review URL(s) modified (diff)

comment:8 Changed 5 months ago by sebastian

  • Status changed from new to reviewing

comment:9 Changed 5 months ago by abpbot

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

comment:10 Changed 5 months ago by jsonesen

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

comment:11 Changed 5 months ago 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 5 months ago by sebastian (previous) (diff)

comment:12 Changed 8 weeks ago 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

Note: See TracTickets for help on using tickets.