Closed Bug 1500213 Opened 6 years ago Closed 6 years ago

Overflow menu changed width in Firefox 63 as a result of menu font inheritance changes

Categories

(Firefox :: Menus, defect)

64 Branch
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox-esr60 --- unaffected
firefox63 --- fix-optional
firefox64 --- affected
firefox65 --- affected

People

(Reporter: ewright, Unassigned)

References

Details

(Keywords: regression)

Attachments

(3 files)

It seems that in 64 the overflow menu width changed from 320px to somewhere around 377px. There are some add-ons that were built for 320px width. 
Add-ons cannot detect if they are being used from the overflow menu, or from the toolbar. We result in the attached picture, add-ons with a very large right space, since they were previously set to 320px.

Can we give add-ons the ability to detect if they are located in the overflow menu, and the width of the menu?
This is basically another version of bug 1373490.

The width of the menu should be detectable the usual way (e.g. media queries), but there are reports in that bug that that doesn't work properly.

I'd mark it as a dupe, but I'm a bit confused by:

(In reply to Erica Wright [:ewright] from comment #0)
> Created attachment 9018350 [details]
> Screen Shot 2018-10-17 at 1.58.49 PM.png
> 
> It seems that in 64 the overflow menu width changed from 320px to somewhere
> around 377px. There are some add-ons that were built for 320px width. 

I'm not aware of deliberate changes here (which doesn't mean they didn't happen). Have you tried to run mozregression? And are you sure this isn't a platform difference? IIRC the width is set in `em` so the px value will depend on platform and/or user font size.
Flags: needinfo?(ewright)
This is 64 vs 62 - I haven't run mozregression yet. but there's definitely been a change - I was asking around UX to try to find when/how it happened, but no one seemed to know of it. I'm starting to think it wasn't intentional.
Flags: needinfo?(ewright)
As I understand it, browserAction panel are supposed to shrink-wrap their contents. So in those cases, the WebExtension is responsible for defining dimensions, and the panel code is responsible for respecting them.

I guess the overflow panel is kind of a special case where, instead, the panel is dictating a minimum width instead of assuming a width based on its contents.

I can see that being pretty confusing for WebExtension authors. We should probably pick a lane here.

I wonder if, for WebExtension subviews, we should close the overflow panel and open the WebExtension panel directly (instead of transitioning to a subview within the same panel), and anchor on the overflow button. That way, I think we'd at least be consistent.

I worry a little bit about adding a class, attribute or some other signal for WebExtension authors to have to worry about in the overflow panel case - that sounds like another hurdle for extension authors to have to deal with.
Attached image max-width.png
(In reply to Mike Conley (:mconley) (:⚙️) from comment #3)

> I guess the overflow panel is kind of a special case where, instead, the
> panel is dictating a minimum width instead of assuming a width based on its
> contents.

it is also dictating a maximum width - you can see this in the max-width.png attached.
Yes, the overflow menu requires a specific width. This is exactly what bug 1373490 and its many dupes are about.

I think the only thing we should investigate here is why there's a change to the width in 63/64 (I think I see the 64-like width on my 63 beta build) when we didn't expect it.

The issue of how to reconcile webextensions wanting to control their popup's width and the overflow menu having its own idea is squarely bug 1373490, and there was substantial discussion there already. Ultimately that's a UX + webextension team call, and so far nobody has made one in that bug, but that's where it should happen.
Summary: Overflow menu, add-on dropdown menus don't fit properly → Overflow menu changed width at some point in 63/64
bug 1481754 changed how font inheritance worked in the overflow panel, which will have changed how many pixels a given em referred to.

In theory we could change the panel width defined at https://searchfox.org/mozilla-central/rev/fcfb479e6ff63aea017d063faa17877ff750b4e5/browser/themes/shared/customizableui/panelUI.inc.css#8 . I suspect macOS may be differently affected here than Windows/Linux because I doubt that there's a significant difference in font size on other platforms, though I could be wrong. Erica, can you confirm if you see the same discrepancy between 62 and 63 on Windows/Linux ?

Dão, thoughts about whether we should do something to make the panel narrower again?
Blocks: 1481754
Flags: needinfo?(ewright)
Flags: needinfo?(dao+bmo)
Summary: Overflow menu changed width at some point in 63/64 → Overflow menu changed width in Firefox 63 as a result of menu font inheritance changes
(In reply to :Gijs (he/him) from comment #7)
> Erica,
> can you confirm if you see the same discrepancy between 62 and 63 on
> Windows/Linux ?

I checked on windows 10, no discrepancy between 64 dev edition and 62 release.
Flags: needinfo?(ewright)
(In reply to :Gijs (he/him) from comment #7)
> bug 1481754 changed how font inheritance worked in the overflow panel, which
> will have changed how many pixels a given em referred to.
> 
> In theory we could change the panel width defined at
> https://searchfox.org/mozilla-central/rev/
> fcfb479e6ff63aea017d063faa17877ff750b4e5/browser/themes/shared/
> customizableui/panelUI.inc.css#8 . I suspect macOS may be differently
> affected here than Windows/Linux because I doubt that there's a significant
> difference in font size on other platforms, though I could be wrong. Erica,
> can you confirm if you see the same discrepancy between 62 and 63 on
> Windows/Linux ?
> 
> Dão, thoughts about whether we should do something to make the panel
> narrower again?

That's entirely a UX question. As you say we're using an em value and it appears that bug 1481754 actually made that work as expected, technically.
Flags: needinfo?(dao+bmo)
(In reply to Dão Gottwald [::dao] from comment #9)
> (In reply to :Gijs (he/him) from comment #7)
> > bug 1481754 changed how font inheritance worked in the overflow panel, which
> > will have changed how many pixels a given em referred to.
> > 
> > In theory we could change the panel width defined at
> > https://searchfox.org/mozilla-central/rev/
> > fcfb479e6ff63aea017d063faa17877ff750b4e5/browser/themes/shared/
> > customizableui/panelUI.inc.css#8 . I suspect macOS may be differently
> > affected here than Windows/Linux because I doubt that there's a significant
> > difference in font size on other platforms, though I could be wrong. Erica,
> > can you confirm if you see the same discrepancy between 62 and 63 on
> > Windows/Linux ?
> > 
> > Dão, thoughts about whether we should do something to make the panel
> > narrower again?
> 
> That's entirely a UX question. As you say we're using an em value and it
> appears that bug 1481754 actually made that work as expected, technically.

So the question is whether from a UX perspective we want to change the width to be narrower again on mac, even if the current width (and this applies not just to the overflow menu, I think) is effectively now more in line with Windows in terms of how much text will fit in the panel at a given font size. Erica, what do you think? (If you're not the right person to make that call, please forward as appropriate. :-) )
Flags: needinfo?(ewright)
Aaron, could you make a call here?
Flags: needinfo?(ewright) → needinfo?(abenson)
It seems a little wide to me but if it's so that we can accommodate potentially long titles, it's probably worth keeping as-is (wider) and more inline with how it looks/works on Windows.
Flags: needinfo?(abenson)
OK, let's close this out per comment #12. We can always reconsider - changing the width of this should (famous last words) be essentially just about changing the one define at https://searchfox.org/mozilla-central/rev/72b1e834f384a2ffec6eb4ce405fbd4b5e881109/browser/themes/shared/customizableui/panelUI.inc.css#8 .
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: