508267 - Assignment to undeclared variable dragAfter warning.

Status: VERIFIED FIXED

(Toolkit :: Toolbars and Toolbar Customization, defect)

Description

[Blake Winton (:bwinton) (:☕️)]

Attachment: A patch to declare the dragAfter variable.

While customizing my toolbar in Thunderbird, I get the following warning:
JavaScript strict warning: chrome://global/content/customizeToolbar.js, line 747: assignment to undeclared variable dragAfter

This patch fixes that.

Comment 1

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

Which version of this file are you patching? That change has already been made to mozilla-central's copy, as far as I can tell:

http://mxr.mozilla.org/mozilla-central/source/toolkit/content/customizeToolbar.js#784

Comment 2

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 392490 [details] [diff] [review]
A patch to declare the dragAfter variable.

Ah, that makes sense.  I'm using the copy from http://mxr.mozilla.org/mozilla1.9.1/source/toolkit/content/customizeToolbar.js#784

(I did do a search for the error message in bugzilla, but nothing showed up.)

Thanks!

Comment 3

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

We can take just that fix on the branch if you'd like.

Comment 4

[Blake Winton (:bwinton) (:☕️)]

I wouldn't bother, since it doesn't really hurt anything, it's just another debug message for me to ignore.  :)

Comment 5

[Phil Ringnalda (:philor)]

I think it's worth taking, just because of the noise involved in "is there anything in the error console?" "yeah, it says 'assignment to undeclared variable dragAfter'" "no, that's known and unrelated, is there anything else?"

Comment 6

[Blake Winton (:bwinton) (:☕️)]

Phil's been here longer than I have, so I'll go with his opinion.

Can we take that fix on the branch please?

Thank you,
Blake.

Comment 7

[Phil Ringnalda (:philor)]

Attachment: Just like the trunk, for 1.9.1

Well, then let's do it exactly like the trunk does (we can't take the patch that fixed it on the trunk, since that was the giant "Switch to using the new drag-drop API" patch).

Drivers: it's not just a silly "fix a warning," since having it undeclared means that if an extension stupidly uses a global named dragAfter, their value will get clobbered, but only in the miserable-to-reproduce case after a user customizes toolbars *and* drags something. There's a microscopic but non-zero chance that someone shipped an extension which is actually intending to use the accidental global dragAfter with its value after customizing (at which point it would be set to something of no use at all to anyone outside that function, so I can't picture why anyone would), but that's extremely unlikely, especially compared to the chances of someone using their own dragAfter global. (And neither use shows up in mxr-test's index of AMO addons, though I wouldn't count on that being up to date.)

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 394903 [details] [diff] [review]
Just like the trunk, for 1.9.1

Approved for 1.9.1.4, a=dveditz for release-drivers

Comment 9

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a5d1f196ea75

Comment 10

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

Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.4pre) Gecko/20090908 Shredder/3.0b4pre ID:20090908033759