Opened 12 months ago

Closed 7 months ago

Last modified 7 months ago

#5354 closed defect (fixed)

Notifications with buttons are not supported in Opera

Reported by: Ross Assignee: jsonesen
Priority: P3 Milestone: Adblock-Plus-3.0-for-Firefox
Module: Platform Keywords:
Cc: kzar, greiner, jsonesen, sebastian Blocked By:
Blocking: Platform: Opera
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29583620/

Description (last modified by sebastian)

Environment

ABP 1.13.2.1785
Opera 36 / 49 / Window 7

How to reproduce

  1. Install ABP (to have default/clear preferences).
  2. Visit http://www.nexusmods.com/games
  3. Repeat Steps 2-3 in Chrome.

Observed behaviour

In Chrome, the anti-adblock notification appears as expected.

In Opera it does not and the following error appears in the extensions background page console:

_generated_background_page.html:1 Unchecked runtime.lastError while running notifications.create: Adding buttons to notifications is not supported.

Expected behaviour

If buttons are not supported, the notification should be displayed without buttons, except if the type of the notification is question in which case the notification should not be shown if buttons are not supported. Either way, no error should be logged.

Change History (13)

comment:1 Changed 12 months ago by Ross

I'm not actually 100% sure if the anti-adblock notification should appear in Opera but the consensus seems to be be it should. The error seems to be related to a platform restriction (not able to add buttons to notifications) so I'm not sure if/how it worked if it did before.

comment:2 Changed 12 months ago by sebastian

  • Cc kzar greiner jsonesen sebastian added
  • Component changed from Unknown to Platform
  • Priority changed from Unknown to P3

Yeah, generally, the notification should appear on Opera. But from reading the error message, I agree, it doesn't seem we can have buttons there, and without buttons this notification seems quite useless. Possibly, we could fallback to HTML notifications though. Either way we should get rid of the error.

comment:3 Changed 11 months ago by kzar

  • Ready set

comment:4 Changed 9 months ago by jsonesen

  • Owner set to jsonesen

comment:5 Changed 9 months ago by jsonesen

Is this duplicated in #5759??

comment:6 Changed 9 months ago by sebastian

  • Summary changed from Anti-adblock notification does not work in Opera to Notifications with buttons are not supported in Opera

comment:7 Changed 9 months ago by sebastian

  • Description modified (diff)

comment:8 Changed 8 months ago by jsonesen

Also, it looks like we will need to implement a html notification if we want them to appear on opera. However, deleting the button property does fix the issues with errors being thrown.

comment:9 Changed 8 months ago by sebastian

  • Description modified (diff)

There is already a fallback to HTML notifications, but it seems HTML notifications cannot have buttons either. So we don't use this fallback for notifications of the type question (e.g. the anti-adblock notification), and not if browser.notifications is supported anyway.

But then again, there is no point in showing the anti-adblock notification if we can not have buttons to opt-in or -out of that list. So if buttons aren't supported in browser.notifications and the notification type is question it seems best to not show the notification at all.

comment:10 Changed 8 months ago by jsonesen

  • Review URL(s) modified (diff)

comment:11 Changed 8 months ago by abpbot

A commit referencing this issue has landed:
Issue 5354 - Adds handling for notifications with buttons in Opera

comment:12 Changed 7 months ago by jsonesen

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

comment:13 Changed 7 months ago by sebastian

  • Milestone set to Adblock-Plus-3.0-for-Chrome-Opera-Firefox
Note: See TracTickets for help on using tickets.