Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#3870 closed change (fixed)

Make legacy options page use asynchronous messaging

Reported by: kzar Assignee: kzar
Priority: P2 Milestone: Adblock-Plus-1.12-for-Chrome-Opera-Safari
Module: Platform Keywords:
Cc: sebastian, greiner, Ross Blocked By: #3889, #3890
Blocking: #3687 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29339314/

Description (last modified by kzar)

Background

We are working at adding experimental support for the new content blocking APIs in Safari >=9 (#3687). Unfortunately however Apple have decided that when the new APIs are used, the old ones are disabled until the extension is restarted. This breaks the legacy options page, and we don't have time to wait for the new options page to be released.

What to change

Make the legacy options page work in Safari without the use of synchronous messaging function canLoad, using the existing asynchronous messaging infrastructure provided by adblockplusui. Ensure that the options page continues to work properly even when the new API is being used.

You will also need to update the adblockplusui dependency to include the following changes:

and the adblockpluscore dependency to include this change:

Hints for testers

We have completely changed the way the legacy options page works behind-the-scenes in this issue. Large parts of the code needed to be removed or rewritten. As far as possible we need to test that the options page continues to function as before, with no regressions on any of the supported platforms.

Some examples of things to try:

  • Managing filters, subscriptions and whitelisted domains from the options page. Also changing them from outside the options page while it's still open to ensure it keeps itself updated.
  • Add a subscription and check it's marked as "Downloading..." then completed / failed. Make sure the last updated dates are added correctly on success.
  • Ensure all the checkboxes still work, and that they are updated when the preferences are modified elsewhere. (For example type require("prefs").Prefs["shouldShowBlockElementMenu"] = false; in the extension's background console and watch to see if the checkbox reacts.)
  • Test places that load the options page to perform an action, for example try clicking on a subscription link. (For example the one at https://testpages.adblockplus.org/ )

But really "just" try everything you can think of!

Change History (18)

comment:1 Changed 4 years ago by sebastian

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

comment:2 Changed 4 years ago by sebastian

  • Summary changed from Make legacy options page work without depreciated APIs in Safari to Make legacy options page use asynchronous messaging

comment:3 Changed 4 years ago by kzar

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

comment:4 Changed 4 years ago by kzar

  • Description modified (diff)

comment:5 Changed 4 years ago by sebastian

  • Blocked By 3887 added

comment:6 Changed 4 years ago by sebastian

  • Blocked By 3887 removed

comment:7 Changed 4 years ago by kzar

  • Blocked By 3889 added

comment:8 Changed 4 years ago by kzar

  • Description modified (diff)

comment:9 Changed 4 years ago by kzar

  • Blocked By 3890 added

comment:10 Changed 4 years ago by kzar

  • Description modified (diff)

comment:11 Changed 4 years ago by kzar

  • Description modified (diff)

comment:12 Changed 4 years ago by abpbot

A commit referencing this issue has landed:
https://hg.adblockplus.org/adblockpluschrome/rev/56182b2d1707

comment:13 Changed 4 years ago by kzar

  • Description modified (diff)
  • Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next
  • Platform changed from Safari to Unknown / Cross platform
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:14 Changed 4 years ago by kzar

  • Cc Ross added

Adding you as CC Ross as this issue is going to need some serious testing!

comment:15 Changed 4 years ago by Ross

So far this seems to break the fixes to #3877 (which was fixed/doesn't occur in 1592).

ABP 1.11.0.1593
Safari 6.0 / OS X 10.8
Safari 7.1.8 / OS X 10.9.5
Safari 8.0.6 / OS X 10.10.3

comment:16 Changed 4 years ago by kzar

The problem you've found looks like a new regression, #3877 was something different. Please could you file an issue for this new regression including steps to reproduce?

comment:17 Changed 4 years ago by Ross

Reported as #3915.

comment:18 Changed 4 years ago by kzar

This caused a regression in old versions of Chrome #4052.

Note: See TracTickets for help on using tickets.