Closed
Bug 1022209
Opened 10 years ago
Closed 10 years ago
msgButton is null error when get new messages button not found on toolbar
Categories
(Thunderbird :: Mail Window Front End, defect)
Thunderbird
Mail Window Front End
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 35.0
People
(Reporter: merike, Assigned: aceman)
References
Details
Attachments
(1 file, 2 obsolete files)
2.36 KB,
patch
|
jsbruner
:
review+
|
Details | Diff | Splinter Review |
JavaScript error: chrome://messenger/content/mailWindowOverlay.js, line 733: msgButton is null That function should check if button was found on toolbar.
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Attachment #8440121 -
Flags: review?(mkmelin+mozilla)
Attachment #8440121 -
Flags: feedback?(gasell+mozilla)
Reporter | ||
Comment 3•10 years ago
|
||
Comment on attachment 8440121 [details] [diff] [review] patch Review of attachment 8440121 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me.
Attachment #8440121 -
Flags: feedback?(gasell+mozilla) → feedback+
Comment 4•10 years ago
|
||
> + // The button has a different ID if it isn't on the toolbar but in the Customize > + // palette. In that case we do not need to update its tooltip. In Thunderbird the toolbarpalette is removed from the DOM but a reference is kept. that's why document.getElementById() can't find it, not that it has a different id. Unless you're talking about Australis+CustomizableUI which Thunderbird doesn't have. > + var usernames = new Set([v.server.username for (v of folders)]); Nit: (var v of folders) or (let v of folders) > + var tooltip = document.getElementById("bundle_messenger") > + var listSeparator = document.getElementById("bundle_messenger") If you're going to change these lines you could at least remove the duplication. var bundle = gMessageNotificationBar.stringBundle; var tooltip = bundle.getString(...) var listSeparator = bundle.getString(...)
(In reply to Philip Chee from comment #4) > > + // The button has a different ID if it isn't on the toolbar but in the Customize > > + // palette. In that case we do not need to update its tooltip. > > In Thunderbird the toolbarpalette is removed from the DOM but a reference is > kept. that's why document.getElementById() can't find it, not that it has a > different id. Unless you're talking about Australis+CustomizableUI which > Thunderbird doesn't have. I probably mixed it with the state when the Customize palette is open. Then the elements are there with an id prefixed with "wrapper-". > > + var usernames = new Set([v.server.username for (v of folders)]); > Nit: (var v of folders) or (let v of folders) Adding the 'let' actually breaks the function. Keeping it without one does not produce the "assigning to uninitialized variable' warnings. > > + var tooltip = document.getElementById("bundle_messenger") > > + var listSeparator = document.getElementById("bundle_messenger") > > If you're going to change these lines you could at least remove the > duplication. Yeah, thanks.
Attachment #8440121 -
Attachment is obsolete: true
Attachment #8440121 -
Flags: review?(mkmelin+mozilla)
Attachment #8443753 -
Flags: review?(mkmelin+mozilla)
Comment 7•10 years ago
|
||
Comment on attachment 8443753 [details] [diff] [review] patch v2 Review of attachment 8443753 [details] [diff] [review]: ----------------------------------------------------------------- LGTM! r=mkmelin
Attachment #8443753 -
Flags: review?(mkmelin+mozilla) → review+
Comment 9•10 years ago
|
||
This doesn't apply cleanly, can it be updated please?
Updated•10 years ago
|
Keywords: checkin-needed
Comment 11•10 years ago
|
||
(In reply to :aceman from comment #10) > Yeah, bitrotted by 967583. I'll update it. Reminder for updating. If you don't have time to unbitrott your patch, I could also do this for you. Luckily this is easy in this case. :-)
Comment 12•10 years ago
|
||
Updated for trunk.
Attachment #8443753 -
Attachment is obsolete: true
Attachment #8479997 -
Flags: review+
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 13•10 years ago
|
||
Thanks guys. Of course I could update it too, but my build is failing for 2 months now and I didn't want to attach the new patch without re-testing. So I'd be glad if anybody could update it and test and attach.
Comment 14•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/b473e9f8b1d9
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 35.0
You need to log in
before you can comment on or make changes to this bug.
Description
•