553539 - Provide a helper function for common toolbar iteration pattern

Status: RESOLVED FIXED

(Toolkit :: Toolbars and Toolbar Customization, enhancement)

Description

[Dão Gottwald [:dao]]

Attachment: patch

Note that I'm using Array.slice(...).forEach( instead of Array.forEach(... so that toolbars can be removed in the callback.

Comment 1

[Philip Chee]

+    for (let i = 0; i < toolbar.childNodes.length; ++i) {
can we use childElementCount here?

+      let item = toolbar.childNodes[i];
will this work? toolbar.children[i]

+      if (item.firstChild && item.firstChild.localName == "menubar")
firstElementChild?

+function forEachToolbar(cb) {
+  Array.slice(gToolbox.childNodes).forEach(function (toolbar) {
+    if (isCustomizableToolbar(toolbar))
+      cb(toolbar);
+  });
+}

How about?
Array.slice(gToolbox.childNodes).filter(isCustomizableToolbar).forEach(cb);

Comment 2

[Philip Chee]

+function forEachToolbar(cb) {
s/cb/callback/g ?

Comment 3

[Dão Gottwald [:dao]]

Why would toolbar.children be better than toolbar.childNodes and firstElementChild better than firstChild? This isn't HTML.

Comment 4

[[e-mail]]

(In reply to comment #1)
> How about?
> Array.slice(gToolbox.childNodes).filter(isCustomizableToolbar).forEach(cb);
Surely if you filter them you don't need to slice first.

Comment 5

[Dão Gottwald [:dao]]

Attachment: patch v2

using filter

Comment 6

[[e-mail]]

Comment on attachment 433543 [details] [diff] [review]
patch v2

>+function forEachToolbar(callback) {
[Is it too obvious to call this forEachCustomizableToolbar?]

Comment 7

[Dão Gottwald [:dao]]

I mused about this as well. It's a bit long, but I guess it's still the better name.

Comment 8

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/15e4e239321a