Notifications crash Edge

[Ross]

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.

[kzar]

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.

[kzar]

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

[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.

[Ross]

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

[kzar]

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.

[abpbot]

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

[kzar]

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

[Ross]

This isn't fixed in the build below yet. Although Sebastian mentioned there should be a 2221 build soon.

ABP 0.9.11.2216 (Devbuild)
Edge 44.17763.1.0 / EdgeHTML 18.17763

[sebastian]

Yes, this was fixed in r2218, but the current devbuild is still at 2216. Build #2223 is currently waiting to get published by Microsoft, but as long as we land new changes in master at a higher rate than the Windows Store processes submissions, we don't get a new devbuild out. ;)

[Ross]

Ah okay, that's useful to know.

This is fixed. Doesn't crash when receiving notifications from the notificationurl or when visiting a site on the adblock warning removal list.

ABP 0.9.11.2223 (Devbuild)
Edge 44.17763.1.0 / EdgeHTML 18.17763