Closed Bug 666790 Opened 13 years ago Closed 13 years ago

addon download progress doorhanger has error when asking permission to install

Categories

(Firefox :: Address Bar, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 7

People

(Reporter: zpao, Assigned: zpao)

Details

Attachments

(1 file)

That summary is less than useful, but take this:

1. Go to https://www.eff.org/https-everywhere
2. Try to install the addon
3. See "Nightly prevented this site from..." dialog
4. Press allow
5. See error in console "Error: this.setProgress is not a function \ Source File: chrome://browser/content/urlbarBindings.xml"

Everything still works. It looks like there are 2 instances of the binding, one created and destroyed before the setTimeout is called. The timeout is never cleared, so it tries to do it's thing on an object that no longer exists. Dave tells me this is because removing a doorhanger in the stack causes the whole thing to be rebuilt so that doesn't sound crazy.
Attached patch Patch v0.1Splinter Review
Makes the error go away. I always forget if setting a property for a binding is acceptable this way. But that's what reviews are for :)
Assignee: nobody → paul
Attachment #541538 - Flags: review?(gavin.sharp)
Attachment #541538 - Flags: review?(gavin.sharp) → review+
As long as you're aware that the property isn't tied to the binding but directly to the node, it's acceptable. Can you call it _updateProgressTimeout instead of _timeout?
Note that I backed this out of mozilla-inbound because it was one of the two changesets most likely to blame for the Mac64 opt test orange.  Hard to tell exactly, since this landed 2 minutes after the other possibly culprit and hence got coalesced with it....
Please don't reland this without considering the change from comment 2.
FYI, I tested both this patch and Bug 665564 against m-c after a merge with m-i after our patches landed, and they both came out fine, so I guess it was some kind of build system freakout causing the problem.  Unless our patches interact in some bizarre fashion. ;)

http://tbpl.mozilla.org/?tree=Try&rev=8c56bee019b6
Re-landed on inbound with the change Dão suggested.
http://hg.mozilla.org/integration/mozilla-inbound/rev/7313d4e3547f
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/7313d4e3547f
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → Firefox 7
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: