Closed Bug 1406738 Opened 2 years ago Closed 2 years ago

Overflow menu is displayed (and empty) when you add a flexible space at the end of the navbar and then make the window smaller

Categories

(Firefox :: Toolbars and Customization, defect, P1)

57 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 58
Tracking Status
firefox57 --- wontfix
firefox58 --- verified

People

(Reporter: 768.jac, Assigned: Gijs)

References

Details

(Whiteboard: [reserve-photon-structure])

Attachments

(2 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171005195903

Steps to reproduce:

1. Add flexible space to the menu bar
2. Resize window to make it not fit
3. Click on the now appearing overflow menu and notice its empty


Actual results:

The overflow menu appears and its empty


Expected results:

It shouldent appear whatsoever. (Unless you're resizing the window so much you're hiding other menu-bar items, in which case they go in there.)

Related to 1383458??? On firefox quantum 57.0b6 64bit
oh and this is easier to reproduce if you have two or more flexible spaces. still possible with one though.
Component: Untriaged → Toolbars and Customization
Blocks: 1392631
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: qe-verify+
Priority: -- → P3
Summary: Overflow menu is displayed (and empty) when flexible spaces dont fit on menu bar → Overflow menu is displayed (and empty) when you add a flexible space at the end of the navbar and then make the window smaller
Whiteboard: [reserve-photon-structure]
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Priority: P3 → P1
Comment on attachment 8917330 [details]
Bug 1406738 - do not set the 'overflowing' attribute if the overflow menu only contains 'special' widgets (like flexible spaces),

https://reviewboard.mozilla.org/r/188338/#review193660

::: browser/components/customizableui/CustomizableUI.jsm:4473
(Diff revision 1)
> +      this._addedListener = false;
>        CustomizableUI.removeListener(this);

Can you swap these two lines so that we don't set _addedListener to false if removeListener throws?

::: browser/components/customizableui/CustomizableUI.jsm:4569
(Diff revision 1)
> +        this._addedListener = false;
>          CustomizableUI.removeListener(this);

Swap these two lines as well.
Attachment #8917330 - Flags: review?(jaws) → review+
FWIW, I think this is enough of an edgecase that we could live without it on 57, and I'm nervous about late changes to the overflow code, but I'm open to other opinions. Jared, what do you think?
Flags: needinfo?(jaws)
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/aca965f290d3
do not set the 'overflowing' attribute if the overflow menu only contains 'special' widgets (like flexible spaces), r=jaws
It's a strange bug. The result of this bug isn't catastrophic, just awkward. How many users will get in to this state? It's unclear but I could see it happening if users want their toolbar icons "centered". Being this late in beta I would prefer to let this get fixed in 58 and not uplift to 57.
Flags: needinfo?(jaws)
wontfixing for 57 per comment 6 & 7. :-)
https://hg.mozilla.org/mozilla-central/rev/aca965f290d3
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Since marked as WONTFIX in 57, marking as Verified for 58 with the following specs:
Windows 10 64bit, Mac OS X 10.13, Ubuntu 16.04 64bit using Nightly 58.0a1 (64-bit).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.