Closed Bug 633571 Opened 9 years ago Closed 9 years ago

Multiple entries in the doorhanger for aborted add-on installations

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla2.0b12
Tracking Status
blocking2.0 --- final+

People

(Reporter: whimboo, Assigned: mossop)

References

Details

(Whiteboard: [softblocker][doorhanger])

Attachments

(1 file)

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b12pre) Gecko/20110209 Firefox/4.0b12pre ID:20110209030359

If you start a download and it gets canceled, multiple entries are visible in the doorhanger. See attachment 507662 [details] as example.

Steps:
1. Open https://www.hskupin.info/mozilla/addons/hash/
2. Click on "Install this Extension"

After step 2 the download has been aborted but the doorhanger still shows the progress meter.
The problem here is that the error notification gets added to the doorhanger which causes all notifications to be rebuilt and the old progress notification destroyed and a new one created. However the new one doesn't verify that it should still be shown because removing itself in the middle of the doorhanger creating all its notifications was causing problems.

I'd like to just make the doorhanger code only create notifications that it needs to but that feels risky at this point, instead just going to have to schedule a check after creation. A little hacky but pretty safe.
blocking2.0: --- → final+
Whiteboard: [softblocker]
Assignee: nobody → dtownsend
Whiteboard: [softblocker] → [softblocker],[doorhanger]
Attached patch patch rev 1Splinter Review
Simple fix, testcase was a little trickier since it seems to be very specific to having a whitelist-blocked install that then redirects to an insecure site.
Attachment #511859 - Flags: review?(gavin.sharp)
Whiteboard: [softblocker],[doorhanger] → [softblocker][doorhanger][has patch][needs review gavin]
Comment on attachment 511859 [details] [diff] [review]
patch rev 1

The multiple executeSoons in the test kind of scare me (are you sure they're sufficient to work reliably?)
Attachment #511859 - Flags: review?(gavin.sharp) → review+
(In reply to comment #3)
> Comment on attachment 511859 [details] [diff] [review]
> patch rev 1
> 
> The multiple executeSoons in the test kind of scare me (are you sure they're
> sufficient to work reliably?)

I can run a few runs to see if I hit any failures but that isn't likely to be anything close to just landing it on tinderbox for a time.

I'm pretty confident that it is correct though, before I added them I predicted that it'd need 2 based on how the code was running and testing showed this to be true.
Comment on attachment 511859 [details] [diff] [review]
patch rev 1

This is a trivial one-line code change and has a very low risk of causing problems.
Attachment #511859 - Flags: approval2.0?
Whiteboard: [softblocker][doorhanger][has patch][needs review gavin] → [softblocker][doorhanger][has patch][needs approval]
Attachment #511859 - Flags: approval2.0? → approval2.0+
Landed: http://hg.mozilla.org/mozilla-central/rev/b5c09f0dce23
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Whiteboard: [softblocker][doorhanger][has patch][needs approval] → [softblocker][doorhanger]
Target Milestone: --- → mozilla2.0b12
Duplicate of this bug: 629510
Depends on: 634661
Depends on: 634680
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b12pre) Gecko/20110216 Firefox/4.0b12pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.