666790 - addon download progress doorhanger has error when asking permission to install

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

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.

Comment 1

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.1

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 :)

Comment 2

[Dão Gottwald [:dao]]

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?

Comment 3

[Boris Zbarsky [:bzbarsky]]

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....

Comment 4

[Dão Gottwald [:dao]]

Please don't reland this without considering the change from comment 2.

Comment 5

[Andrew McCreight [:mccr8]]

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

Comment 6

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Re-landed on inbound with the change Dão suggested.
http://hg.mozilla.org/integration/mozilla-inbound/rev/7313d4e3547f

Comment 7

[Joe Drew (not getting mail)]

http://hg.mozilla.org/mozilla-central/rev/7313d4e3547f