Opened on 12/01/2016 at 11:57:03 AM
Closed on 01/12/2017 at 07:41:24 AM
Last modified on 10/08/2019 at 05:50:11 PM
#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/ |
||
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.
Attachments (0)
Change History (7)
comment:1 Changed on 12/01/2016 at 11:59:27 AM by abpbot
comment:2 Changed on 12/01/2016 at 12:00:40 PM 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 on 12/09/2016 at 10:11:49 AM 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:5 Changed on 01/12/2017 at 07:37:52 AM by abpbot
A commit referencing this issue has landed:
Issue 4680 - Fixed regression with showOptions callback
comment:6 Changed on 01/12/2017 at 07:41:24 AM by kzar
- Resolution set to fixed
- Status changed from reopened to closed
comment:7 Changed on 07/29/2019 at 07:47:02 AM by jha2125
spam
A commit referencing this issue has landed:
Issue 4680 - Use runtime.openOptionsPage when available