Closed Bug 1025547 Opened 6 years ago Closed 5 years ago

newmailaccount/jquery.tmpl.js, line 123: reference to undefined property def[1]

Categories

(Thunderbird :: Account Manager, defect)

x86
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 40.0

People

(Reporter: aceman, Assigned: aceman)

References

Details

Attachments

(1 file)

STR:
1. open the New -> Get New Account Wizard
2. input name, click Search

Result:
JavaScript strict warning:
chrome://messenger/content/newmailaccount/jquery.tmpl.js, line 123:
reference to undefined property def[1]

(and a ton of jquery messages)
Attached patch patchSplinter Review
Do not reference def[1] if there is no $2 to substitute.
Attachment #8440326 - Flags: review?(mconley)
Attachment #8440326 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8440326 [details] [diff] [review]
patch

Review of attachment 8440326 [details] [diff] [review]:
-----------------------------------------------------------------

Didn't try but can't you just use 

fnargs || (def.length > 1 && def[1]))
I think that will hide a bug.
In my patch, if there is $2 we need either fnargs or def[1]. If none of those has a value, we get the warning again, which is good, as the caller is buggy.
In your suggestion we just silently use "undefined" or whatever is the result of that expression.
Well undefined should be fine. It's first splitting on $2, then joining the parts. But, you'd only have one part since $2 is missing so it doesn't matter what you pass to join().
Comment on attachment 8440326 [details] [diff] [review]
patch

Review of attachment 8440326 [details] [diff] [review]:
-----------------------------------------------------------------

Instead of forking an old version of jquery.tmpl.js, does this bug go away if we update to the latest version of this library (which should be compatible with the version of jQuery that we've got under mail/)
Attachment #8440326 - Flags: review?(mconley)
How do we update jquery?
Flags: needinfo?(acelists)
You'd replace the file with https://github.com/BorisMoore/jquery-tmpl ... but it's not like that's maintained. So, I guess we could just patch our copy, not to cause other side effects and update needs elsewhere.
Feels weird to fork a jQuery library. But I guess it's small enough that it's not a big deal.
Comment on attachment 8440326 [details] [diff] [review]
patch

Review of attachment 8440326 [details] [diff] [review]:
-----------------------------------------------------------------

Cancelling r request per previous comments
Attachment #8440326 - Flags: review?(mkmelin+mozilla)
If bug 1014609 is finished, this one becomes irrelevant.
Depends on: 1014609
As per comment #10 and https://hg.mozilla.org/comm-central/rev/349f3470bcce this file no longer exists.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 40.0
You need to log in before you can comment on or make changes to this bug.