#4680 closed change (fixed)

Use runtime.openOptionsPage when available

Reported by: kzar Assignee: kzar
Priority: P3 Milestone: Adblock-Plus-1.13-for-Chrome-Opera
Module: Platform Keywords:
Cc: sebastian, trev Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29366526/
https://codereview.adblockplus.org/29367157/

Description (last modified by kzar)

Background

With Chrome 42 a new API was added chrome.runtime.openOptionsPage which we want to make use of. (Firefox's WebExtensions API also supports it, and actually does not support all the APIs our previous implementation used.)

Unfortunately Edge (tested version 38) does not yet contain the corresponding browser.runtime.openOptionsPage and so the workaround will have to stay for now. (Since with issue #4579 we are moving from using the Chrome APIs to targeting browserext.)

What to change

Change chrome/ext/background.js to use chrome.runtime.openOptionsPage if available, otherwise use our existing implementation. Add a comment that the existing implementation is now only required for Edge support, and mention the version tested.

Hints for testers

Ensure the options page still opens correctly in Adblock Plus for Chrome. Test edge cases such as opening the options page when it's already open.

Also test that the option page opens correctly and highlights the notification settings section when opened from a notification. To trigger a suitable notification see my notification testing hints.

It's important that you test everything with both an old (version 49) and modern (version 55) of Chrome.

Change History (6)

comment:1 Changed 12 months ago by abpbot

A commit referencing this issue has landed:
Issue 4680 - Use runtime.openOptionsPage when available

comment:2 Changed 12 months ago by kzar

  • Description modified (diff)
  • Milestone set to Adblock-Plus-for-Chrome-Opera-next
  • Resolution set to fixed
  • Status changed from new to closed

comment:3 Changed 12 months ago by kzar

  • Description modified (diff)
  • Resolution fixed deleted
  • Review URL(s) modified (diff)
  • Status changed from closed to reopened

(As Sebastian pointed out there was a regression with my last change, I forgot to test the callback which is used when the options page is opened by a notification. During the process of fixing that I actually found another bug which would have broken that functionality anyway.)

comment:4 Changed 11 months ago by kzar

  • Description modified (diff)

comment:5 Changed 11 months ago by abpbot

A commit referencing this issue has landed:
Issue 4680 - Fixed regression with showOptions callback

comment:6 Changed 11 months ago by kzar

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.