Closed Bug 589954 Opened 12 years ago Closed 12 years ago

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

Categories

(Firefox :: General, defect)

defect
Not set
major

Tracking

()

VERIFIED FIXED
Firefox 4.0b5
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: u88484, Assigned: Gavin)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
blocking2.0: --- → ?
I suspect this would be caused by bug 572967
Blocks: 572967
blocking2.0: ? → betaN+
Summary: Doorhanger notification only shows once if focus is stolen → new addon-install-blocked notification doesn't replace existing (potentially dismissed) notification
Assignee: nobody → gavin.sharp
OS: Windows 7 → All
Hardware: x86_64 → All
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.
Keywords: regression
Duplicate of this bug: 590907
Note that this is true for all the add-on notifications.
Summary: new addon-install-blocked notification doesn't replace existing (potentially dismissed) notification → new addon notifications don't replace existing (potentially dismissed) notification
Attached patch patchSplinter Review
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.
Attachment #469668 - Flags: review?(dtownsend)
Component: Add-ons Manager → General
Product: Toolkit → Firefox
QA Contact: add-ons.manager → general
Attachment #469668 - Flags: review?(dtownsend) → review+
https://hg.mozilla.org/mozilla-central/rev/1c0313114a8f
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b5
Verified fixed
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100828 Firefox/4.0b5pre
Status: RESOLVED → VERIFIED
Gavin, which parts could be automatically tested and which would require manual tests?
Flags: in-testsuite?
Flags: in-litmus?
This is has automated tests.
Flags: in-testsuite? → in-testsuite+
...and I don't think it needs a litmus test.
Flags: in-litmus?
You need to log in before you can comment on or make changes to this bug.