860093 - Regression from Bug 824150: toolbox has no method getElementById()

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, defect)

Description

[alta88]

there's no getElementById on an element (toolbox) so this is throwing, maybe even preventing the after customize event..

diff --git a/mail/base/content/mailCore.js b/mail/base/content/mailCore.js
--- a/mail/base/content/mailCore.js
+++ b/mail/base/content/mailCore.js
@@ -204,28 +204,28 @@ function MailToolboxCustomizeDone(aEvent
 
   var customizePopup = document.getElementById(customizePopupId);
   customizePopup.removeAttribute("disabled");
 
   // make sure our toolbar buttons have the correct enabled state restored to them...
   if (this.UpdateMailToolbar != undefined)
     UpdateMailToolbar(focus);
 
-  var toolbox = document.getElementsByAttribute("doCustomization", "true")[0];
+  let toolbox = document.querySelector('[doCustomization="true"]');
   if (toolbox) {
     toolbox.removeAttribute("doCustomization");
 
     // The GetMail button is stuck in a strange state right now, since the
     // customization wrapping preserves its children, but not its initialized
     // state. Fix that here.
     // Fix Bug 565045: Only treat "Get Message Button" if it is in our toolbox
-    var popup = toolbox.getElementsByAttribute("id", "button-getMsgPopup")[0];
+    let popup = toolbox.getElementById("button-getMsgPopup");
     if (popup) {
       // We can't use _teardown here, because it'll remove the Get All menuitem
-      let sep = toolbox.getElementsByAttribute("id", "button-getAllNewMsgSeparator")[0];
+      let sep = toolbox.getElementById("button-getAllNewMsgSeparator");
       while (popup.lastChild != sep)
         popup.removeChild(popup.lastChild);
     }
   }
 }

Comment 1

[:aceman]

Thanks for catching this.
The original getElementsByAttribute is a method of toolbox, so the change just needs to be reverted (maybe a comment added).

Comment 2

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: patch, v1

Now using querySelector.

Comment 3

[:aceman]

Aryx, can you try if the patch also fixes bug 859287?

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

It also fixes bug 859287, sorry for the regression.

Comment 6

[Josiah Bruner [:jsbruner]]

(In reply to Archaeopteryx [:aryx] from comment #5)
> It also fixes bug 859287, sorry for the regression.

Nope. It does not fix bug 859287. So it is not a regression. Or at least not the entire reason.

Comment 7

[:aceman]

Comment on attachment 735673 [details] [diff] [review]
patch, v1

Maybe mconley could look at this simple thing faster.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 735673 [details] [diff] [review]
patch, v1

Definitely yes. r=me, no need to bother Mark with this one.

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: patch, v2, r=mconley

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/a252686c9393

Standard practice throughout the project is to resolve bugs when they hit {comm,mozilla}-central and use the status flags for tracking branch uplifts. Please put checkin-needed back on the bug when it gets approval for aurora uplift.

Comment 11

[Mark Banner (:standard8)]

Comment on attachment 737174 [details] [diff] [review]
patch, v2, r=mconley

[Triage Comment]
If the patch is ready, and you want to push this to aurora (or other branch) the correct way is to signal approval-comm-<branch> (set to ?) on the patch.

Then we can come along a approve it. a=me since this is a low risk fix for a regression.

Comment 12

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-aurora/rev/c166eba39a7b