589954 - new addon notifications don't replace existing (potentially dismissed) notification

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[u88484]

Doorhanger notification only shows once if focus is stolen by not interacting with the notification and clicking somewhere on the page or switching tabs.

STR:
1) Load http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1282596643/
2) Click on the firefox-4.0b4pre.en-US.langpack.xpi link (doorhanger notification popups. Don't interact with the doorhanger notification at all)
3) Switch tabs or click on the page
4) Go back to the original tab and click on the link again

No notification is shown again, the user is left clueless on why nothing happens when the link is clicked.

Comment 1

[Dave Townsend [:mossop]]

I suspect this would be caused by bug 572967

Comment 2

[u88484]

This is technically a regression (even though doorhanger notifications are new) due to the fact that the notification bar persists if tabs are switched on Firefox 3.6  Tab tearing wasn't in Firefox 3.6 so not a regression on that part.

Comment 4

[Dave Townsend [:mossop]]

Note that this is true for all the add-on notifications.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

This removes the checks for an existing notification in the xpinstall-blocked/disabled cases. getNotification() returning non-null does not necessarily mean that the notification is visible, as it does with the notification box API (it could be existent but "dismissed"). Re-show()ing them makes sure they're visible.

It also removes the code that explicitly removes the old notification for addon-install-complete, since PopupNotifications.show() already handles that (it replaces existing notifications with the same ID).

test_renotify_installed already passes, because of the addon-install-complete code I'm removing, but I added it to ensure that the removal is OK. test_renotify_blocked covers the change made here (fails without this patch). I haven't pushed to try yet because it is busted, but will do that tomorrow.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/mozilla-central/rev/1c0313114a8f

Comment 7

[u88484]

Verified fixed
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100828 Firefox/4.0b5pre

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Gavin, which parts could be automatically tested and which would require manual tests?

Comment 9

[:Gavin Sharp [email: [e-mail]]]

This is has automated tests.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

...and I don't think it needs a litmus test.