Once we have a permanent/sticky part to the overflow panel, we'll need to ensure we always show the overflow button if there are items in that part of the overflow panel.

Comment on attachment 8866411 [details] Bug 1354094 - show the overflow button when there are items in the permanent overflow menu, https://reviewboard.mozilla.org/r/138044/#review141196 With the comments below fixed, it'll be great - but I'd still like to confirm it all and try the patch locally as well. ::: browser/components/customizableui/CustomizableUI.jsm (Diff revision 1) > child.setAttribute("overflowedItem", true); > child.setAttribute("cui-anchorid", this._chevron.id); > CustomizableUIInternal.notifyListeners("onWidgetOverflow", child, this._target); > > this._list.insertBefore(child, this._list.firstChild); > - if (!this._toolbar.hasAttribute("overflowing")) { Please take a look at the hg blame, because I've got the feeling that this was put here intentionally. ::: browser/components/customizableui/CustomizableUI.jsm:4277 (Diff revision 1) > let win = this._target.ownerGlobal; > win.UpdateUrlbarSearchSplitterState(); > > if (!this._collapsed.size) { > this._toolbar.removeAttribute("overflowing"); > - CustomizableUI.removeListener(this); > + this._panel.setAttribute("nooverfloweditems", "true"); Same here, really. Of course, setting the attribute should stay. ::: browser/components/customizableui/CustomizableUI.jsm:4367 (Diff revision 1) > aNode.removeAttribute("overflowedItem"); > CustomizableUIInternal.notifyListeners("onWidgetUnderflow", aNode, this._target); > > if (!this._collapsed.size) { > this._toolbar.removeAttribute("overflowing"); > - CustomizableUI.removeListener(this); > + this._panel.setAttribute("nooverfloweditems", "true"); Same. ::: browser/components/customizableui/content/panelUI.inc.xul:378 (Diff revision 1) > <panel id="widget-overflow" > role="group" > type="arrow" > noautofocus="true" > position="bottomcenter topright" > + nooverfloweditems="true" Can we use a positive-named attribute name and have the default of 'not present' be reflective of the initial state? So I'd have opted for 'containsoverfloweditems' or 'hasoverfloweditems' or something. Perhaps even include the dynamic property of these items in there: 'hasdynamicoverfloweditems'... ::: browser/components/customizableui/content/panelUI.js:89 (Diff revision 1) > _initPhotonPanel() { > if (gPhotonStructure) { > this.overflowFixedList.hidden = false; > this.overflowFixedList.nextSibling.hidden = false; > CustomizableUI.registerMenuPanel(this.overflowFixedList, CustomizableUI.AREA_FIXED_OVERFLOW_PANEL); > + this.navbar.setAttribute("photon-structure", "true"); It doesn't seem you're using this attribute in this patch. Can you remove it? ::: browser/components/customizableui/content/panelUI.js:605 (Diff revision 1) > onPhotonChanged() { > this.reinit(); > }, > > + updateOverflowStatus() { > + let haveKids = this.overflowFixedList.hasChildNodes(); nit: s/haveKids/hasKids/ ::: browser/components/customizableui/content/panelUI.js:607 (Diff revision 1) > }, > > + updateOverflowStatus() { > + let haveKids = this.overflowFixedList.hasChildNodes(); > + if (haveKids && !this.navbar.hasAttribute("nonemptyoverflow")) { > + this.navbar.setAttribute("nonemptyoverflow", "true"); nit: `true.toString() == "true"`, so I usually use the bool variant.

Comment on attachment 8866411 [details] Bug 1354094 - show the overflow button when there are items in the permanent overflow menu, https://reviewboard.mozilla.org/r/138044/#review141196 > Can we use a positive-named attribute name and have the default of 'not present' be reflective of the initial state? > > So I'd have opted for 'containsoverfloweditems' or 'hasoverfloweditems' or something. Perhaps even include the dynamic property of these items in there: 'hasdynamicoverfloweditems'... done, using 'hasdynamicitems' because we put it on the 'widget-overflow' panel so the CSS rule is clear like this. > It doesn't seem you're using this attribute in this patch. Can you remove it? It's used in the browser.css change. :-)

Comment on attachment 8866411 [details] Bug 1354094 - show the overflow button when there are items in the permanent overflow menu, https://reviewboard.mozilla.org/r/138044/#review141328 Looking great, ship it!

Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0645deeb4caa show the overflow button when there are items in the permanent overflow menu, r=mikedeboer

Verified on Windows, Mac, and Ubuntu.