Opened 16 months ago

Closed 11 months ago

Last modified 11 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 16 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 16 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 16 months ago by kzar

  • Ready set

comment:4 Changed 13 months ago by jsonesen

  • Owner set to jsonesen

comment:5 Changed 13 months ago by jsonesen

Is this duplicated in #5759??

comment:6 Changed 13 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 13 months ago by sebastian

  • Description modified (diff)

comment:8 Changed 12 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 12 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 12 months ago by jsonesen

  • Review URL(s) modified (diff)

comment:11 Changed 12 months ago by abpbot

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

comment:12 Changed 11 months ago by jsonesen

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

comment:13 Changed 11 months ago by sebastian

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