1890560 - Replacing notification with tag fires events with different order on Windows; show happens before close

Status: RESOLVED FIXED

(Core :: DOM: Notifications, defect, P3)

Description

[Kagami Rosylight [:saschanaz] (they/them)]

new Notification("foo", { tag: "foo" }).onclose = console.log;
new Notification("bar", { tag: "foo" }).onshow = console.log;

With the above code, close fires before show on all other browsers/platforms: Firefox on macOS/LinuxXUL, Chrome on Windows/macOS/Linux, and Safari. Only Firefox on Windows fires close event first.

Perhaps this depends on the OS behavior, but I'd like to see why we behave different when Chrome does not.

Comment 1

[Kagami Rosylight [:saschanaz] (they/them)]

Because we just unregister the old handler late: https://searchfox.org/mozilla-central/rev/e85232b4b28ecc970240d39203e417d1c320623c/widget/windows/ToastNotification.cpp#517

Comment 2

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: Bug 1890560 - Call UnregisterHandler earlier for the old handler r=asuth!,nrishel!,nalexander!

This makes sure we get the consistent behavior for every platform (except Android for now).

Comment 3

[Kagami Rosylight [:saschanaz] (they/them)]

Actually it turns out the libnotify backend is also bugged while the XUL implementation works fine.

Comment 4

[Kagami Rosylight [:saschanaz] (they/them)]

Err, it looks like libnotify uses sync call to dbus and thus has no success callback, while Chrome uses async call and thus has one. Meaning we can't get close callback while creating notification. 🤔

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

Does that also mean we're synchronously blocking the parent process main thread when doing that too?

Comment 6

[Kagami Rosylight [:saschanaz] (they/them)]

Unfortunately yes.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a0dbaa387380
Call UnregisterHandler earlier for the old handler r=asuth,nalexander,win-reviewers,gstoll

Comment 8

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/50761 for changes under testing/web-platform/tests

Comment 9

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/a0dbaa387380

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot