Opened 14 months ago

Closed 14 months ago

Last modified 14 months ago

#7206 closed defect (fixed)

Notifications crash Edge — at Version 11

Reported by: Ross Assignee: kzar
Priority: P2 Milestone: Adblock-Plus-3.4.3-for-Chrome-Opera-Firefox
Module: Platform Keywords: externaldependency
Cc: sebastian, kzar, geo Blocked By:
Blocking: Platform: Edge
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://gitlab.com/eyeo/adblockplus/adblockpluschrome/merge_requests/25

Description (last modified by kzar)

Environment

ABP 0.9.11.2214 (Devbuild), or 0.9.11 (Release)
Edge 44.17763.1.0 / EdgeHTML 18.17763

How to reproduce

Intercept and rewrite the request to notification.adblockplus.org using a proxy to return a notification file.

OR

Append this snippet to adblockpluschrome/lib/csp.js, rebuild and reload the extension, then navigate to https://example.com:

require("notification").Notification.addNotification({
  id: new Date() | 0,
  type: "critical",
  title: "Notification title",
  message: "<a>Open contribution page</a>",
  links: ["contribute"],
  urlFilters: ["||example.com^$document"]
});

Observed behaviour

Edge crashes just after ABP makes the request for the notification file. The same notification file works fine in Chrome.

Expected behaviour

Notifications to not crash the browser.

Notes

Even simply calling the notification API from the extension's background console crashes Edge consistently! For example, try this snippet:

browser.notifications.create({
  type: "basic",
  iconUrl: "icons/abp-16.png",
  title: "crash test",
  message: "hello world"
});

See the related Microsoft Edge bug report: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/20146233/

Hints for testers

As a workaround we've stopped using the browser.notifications API on Edge. Ensure that notifications appear in the popup window instead on Edge, and that notifications still show up as normal on other platforms.

Change History (12)

comment:1 Changed 14 months ago by kzar

  • Component changed from Unknown to Platform
  • Description modified (diff)
  • Priority changed from Unknown to P2
  • Ready set

comment:2 Changed 14 months ago by kzar

  • Description modified (diff)

comment:3 Changed 14 months ago by kzar

  • Description modified (diff)
  • Keywords externaldependency added

Edge crashes when we use the browser.notifications.create API, it happens with the currently released version of Adblock Plus too. I've reported this to Microsoft, but I don't think this should block our release.

comment:4 Changed 14 months ago by kzar

Since apparently the Edge issue tracker is also having troubles, I'll attach a copy of my report here too...

comment:5 follow-up: Changed 14 months ago by sebastian

What happens when navigating to a website targeted by the Adblock Warning Removal List? If this is causing Microsoft Edge to crash, we might want to address this issue before the release. Though not a regression, this would be quite critical. I suppose, if nothing else, we could just make sure to not use the notifications API on Microsoft Edge.

Last edited 14 months ago by sebastian (previous) (diff)

comment:6 in reply to: ↑ 5 Changed 14 months ago by Ross

Can confirm it also crashes when just visiting a site targeted by the Adblock Warning Removal List (and triggering the notification).

comment:7 Changed 14 months ago by kzar

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

I suppose, if nothing else, we could just make sure to not use the notifications API on Microsoft Edge.

I guess you're right, we should probably just avoid using browser.notifications entirely on Edge until Microsoft sort this out.

comment:8 Changed 14 months ago by kzar

  • Owner set to kzar

comment:9 Changed 14 months ago by abpbot

A commit referencing this issue has landed:
Issue 7206 - Avoid using browser.notifications API on Edge!

comment:10 Changed 14 months ago by kzar

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
  • Resolution set to fixed
  • Status changed from reviewing to closed

(I don't see a Adblock-Plus-For-Edge-next milestone, so hopefully this milestone is OK.)

comment:11 Changed 14 months ago by kzar

  • Description modified (diff)
Note: See TracTickets for help on using tickets.