Opened on 03/18/2018 at 11:08:01 PM
Closed on 03/19/2018 at 05:42:38 PM
Last modified on 05/14/2018 at 08:59:47 AM
#6496 closed defect (fixed)
Account for browsers that don't support notifications with buttons
| Reported by: | sebastian | Assignee: | sebastian |
|---|---|---|---|
| Priority: | P2 | Milestone: | Adblock-Plus-3.1-for-Chrome-Opera-Firefox |
| Module: | Platform | Keywords: | |
| Cc: | greiner | Blocked By: | |
| Blocking: | Platform: | Unknown / Cross platform | |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): |
https://codereview.adblockplus.org/29726570 |
||
Description (last modified by sebastian)
Background
- Firefox throws a synchronous error when calling notifications.create({buttons: ...}).
- Opera responds with an asynchrnous error when calling notifications.create({buttons: ...}).
- Microsoft Edge doesn't support the notifications API at all (however, this case is handled).
This has been partially addressed in #5354 by not showing notifications of the type question (there is no point if there cannot be buttons), and removing any buttons from other kind of notifications, on Opera.
However, since Firefox throws synchronously, the error there isn't handled currently, causing error messages in the background page, and preventing any kind of notification from being shown on Firefox.
Furthermore, there is not much of a point in subscribing users (on installation) to the "Adblock Warning Removal List" if they cannot see the related notification anyway.
What to change
- Handle the error thrown on Firefox, with the same fallback we have on Opera, i.e. ignore notifications of the type question and retry to show other notifications without buttons.
- Don't subscribe users on browsers that don't support notifications with buttons (and therefore will ignore question notifications), by default, to the "Adblock Warning Removal List".
Attachments (0)
Change History (8)
comment:1 Changed on 03/18/2018 at 11:11:16 PM by sebastian
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:3 Changed on 03/19/2018 at 11:23:42 AM by greiner
- Cc greiner added
comment:4 Changed on 03/19/2018 at 05:41:25 PM by abpbot
comment:5 Changed on 03/19/2018 at 05:42:38 PM by sebastian
- Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
- Resolution set to fixed
- Status changed from reviewing to closed
comment:7 Changed on 04/25/2018 at 02:36:00 PM by abpbot
A commit referencing this issue has landed:
Issue 6496 - Account for notifications.create() failing asynchronously
comment:8 Changed on 05/14/2018 at 08:59:47 AM by Ross
- Tester changed from Unknown to Ross
- Verified working set
Done. Notifications appear to be working as expected. Question notifications still work in browsers that support them (Chrome).
ABP 3.0.4.2044
Firefox 59 / 55 / 51 / Windows 10
Chrome 66 / 58 / 49 / Windows 7
Opera 52 / 45 / 36 / Windows 10
A commit referencing this issue has landed:
Issue 6496 - Account for browsers that don't support notifications with buttons