598934 - Addon Bar should be bottom of Toolbar contextual menu, currently it's top

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Paul [pwd]]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b7pre) Gecko/20100923 Firefox/4.0b7pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; rv:2.0b7pre) Gecko/20100923 Firefox/4.0b7pre

Currently should you right click on the Navigation bar, you'll see "Addon Bar" at the top. Above Menu Bar, Navigation Bar and Bookmarks Toolbar. It should be at the bottom of that list.

Reproducible: Always

Comment 1

[Dietrich Ayala (:dietrich)]

Attachment: v1

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 479774 [details] [diff] [review]
v1

>+  let addonBar = null;
>   let toolbarNodes = [document.getElementById("addon-bar")];
>   for (i = 0; i < gNavToolbox.childNodes.length; ++i)
>     toolbarNodes.push(gNavToolbox.childNodes[i]);
>   toolbarNodes.forEach(function(toolbar) {

Seems like this should just be something like this:

let toolbarNodes = Array.slice(gNavToolbox.childNodes);
toolbarNodes.push(document.getElementById("addon-bar"));

Comment 3

[Dietrich Ayala (:dietrich)]

not sure if slice() will work there because childNodes is a nodelist not a regular js array. but i'll give it a try!

Comment 4

[Dietrich Ayala (:dietrich)]

Attachment: v1.1

works!

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 480071 [details] [diff] [review]
v1.1

With the slice/push change, why is the rest of this patch needed? r- because this doesn't seem to make sense, please re-request if I'm missing something.

Comment 6

[Dietrich Ayala (:dietrich)]

Attachment: v1.2

right, not needed

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 480117 [details] [diff] [review]
v1.2

>-  var i;
>-
>   // Empty the menu
>-  for (i = popup.childNodes.length-1; i >= 0; --i) {
>+  for (var i = popup.childNodes.length-1; i >= 0; --i) {

You could replace var with let while you're at it.

Comment 8

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/286e78ac1ba1