Closed Bug 1399963 Opened 7 years ago Closed 7 years ago

Font name in Preferences is Indistinguishable

Categories

(Firefox :: Settings UI, defect)

57 Branch
Unspecified
Windows 10
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 57
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 + verified
firefox58 --- verified

People

(Reporter: alice0775, Assigned: myk)

References

Details

(Keywords: regression, Whiteboard: [photon-preferences])

Attachments

(7 files)

Attached image screenshot
Build identifier:
Build ID 20170914100122
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0

Reproducible : always

Steps To Reproduce:
1. Open Preferences
2. Click select box of font list


Actual Results:
See screenshot

Font name is too short, indistinguishable.
(In reply to Richard Marti (:Paenglab) from comment #1)
> This could be because of this change:
> https://hg.mozilla.org/mozilla-central/rev/f255ec4e8c36#l4.16

Yep, even en-US build, the font name in the list is truncated after landing Bug 1375978.
Blocks: 1375978
Summary: Font name in Preferences is Indistinguishable in ja locale build → Font name in Preferences is Indistinguishable
This is a regression from this width rule I added in bug 1375978 to prevent the "default font" menulist in the General pane from resizing after async font enumeration completes:

http://searchfox.org/mozilla-central/rev/6326724982c66aaeaf70bb7c7ee170f7a38ca226/browser/themes/shared/incontentprefs/preferences.inc.css#435-437

The menulists in the Fonts dialog also have that rule, f.e.:

http://searchfox.org/mozilla-central/rev/6326724982c66aaeaf70bb7c7ee170f7a38ca226/browser/components/preferences/fonts.xul#105

But they additionally have a min-width rule to ensure they're wide enough:

http://searchfox.org/mozilla-central/rev/6326724982c66aaeaf70bb7c7ee170f7a38ca226/browser/themes/shared/incontentprefs/preferences.inc.css#439-444

The "default font" menulist doesn't have that rule, and adding (or making the menu flexible) it would resolve this problem, I think.  I'll test that and submit a patch shortly.
Assignee: nobody → myk
Status: NEW → ASSIGNED
The old "default font" menulist dynamically adjusted its width to its contents, which was fine when the menuitems were inserted before the page was painted.  Now that insertion happens asynchronously, dynamic adjustment of width would cause the menulist to "jump" shortly after the page is loaded, which would be distracting and disruptive.

My original fix for this was to set the menulist's width to 0 to force it to stay the same width, but that makes it too narrow for many font names (because it becomes as narrow as possible).  The Fonts dialog resolves this problem by setting a reasonable width for its menulists.  This patch does the same for the "default font" menulist.

Note: the Fonts dialog menulists are 30ch wide, but that width would eliminate the space between the "size" menulist and the "Advanced…" button; so this patch makes the "default font" menulist 25ch wide.

(If we didn't care about that space, we could simply remove the <spacer flex="1" /> in it and let the menulist flex to take up all available space.  But presumably that spacer is there for a reason, as it does look better to have some space there.

Also, setting an explicit width prevents the "default font" menulist from growing too wide in the event that a user has an extra-long font name installed on their system, which was a problem with the old implementation.)
Attachment #8908352 - Flags: review?(nhnt11)
Considering the case of extra-long font name, is it possible to add a tooltip?
(In reply to Alice0775 White from comment #6)
> Considering the case of extra-long font name, is it possible to add a
> tooltip?

It's possible, via a change like this, although it adds tooltips to all menuitems.  Note that 25ch is actually wider than the default font menulist used to be, so it'll result in fewer names being elided than before, fully resolving the regression (and then some).

That being said, tooltips would certainly enhance a user's ability to see the full names of all fonts.  On my main development machine (Mac, en-US), even with the first patch in this bug, there are fonts whose names are elided, including Avenir Next Condensed, Bordeaux Roman Bold LET, and Devanagari Sangam MN.
Attachment #8908396 - Flags: feedback?(nhnt11)
Here's what it looks like with both patches applied on Windows (en-US locale): a wider menulist and a tooltip when hovering over a menuitem.
Comment on attachment 8908352 [details] [diff] [review]
set a wider explicit width on the "default font" menulist

Review of attachment 8908352 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8908352 - Flags: review?(nhnt11) → review+
Comment on attachment 8908396 [details] [diff] [review]
add tooltips to default font menuitems

Review of attachment 8908396 [details] [diff] [review]:
-----------------------------------------------------------------

I think the objective accessibility improvement for long font names is worth the redundant tooltip for smaller ones.
Attachment #8908396 - Flags: feedback?(nhnt11) → feedback+
Comment on attachment 8908396 [details] [diff] [review]
add tooltips to default font menuitems

(In reply to Nihanth Subramanya [:nhnt11] from comment #10)

> I think the objective accessibility improvement for long font names is worth
> the redundant tooltip for smaller ones.

Ok, then I think this patch is also ready for review! ;-)
Attachment #8908396 - Flags: review?(nhnt11)
Comment on attachment 8908396 [details] [diff] [review]
add tooltips to default font menuitems

Review of attachment 8908396 [details] [diff] [review]:
-----------------------------------------------------------------

Hmm, so I thought about this more, and here's another idea: if we change the `sizetopopup` attribute value of the menulist to `none` instead of `pref`, the popup's width will not be constrained to the width of the menu (https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Attribute/sizetopopup).

What do you think about doing this, along with a reasonable max-width on the menupopup for the edge case in which the user has a ridiculously-long-named font installed? This might even let us get away with a smaller width value for the menu, since opening the popup will make it immediately obvious what font is selected.
Attachment #8908396 - Flags: review?(nhnt11)
Hmm, what I suggested in comment 12 looks fine on MacOS but I have a feeling it might look ugly on Windows, since the popup is currently perfectly aligned with the menu right now and we'd lose that alignment with sizetopopup="none".
(In reply to Nihanth Subramanya [:nhnt11] from comment #12)

> Hmm, so I thought about this more, and here's another idea: if we change the
> `sizetopopup` attribute value of the menulist to `none` instead of `pref`,
> the popup's width will not be constrained to the width of the menu
> (https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Attribute/
> sizetopopup).
> 
> What do you think about doing this, along with a reasonable max-width on the
> menupopup for the edge case in which the user has a ridiculously-long-named
> font installed? This might even let us get away with a smaller width value
> for the menu, since opening the popup will make it immediately obvious what
> font is selected.

sizetopopup="none" does increase the width of the menupopup on my Mac, although it doesn't do so enough to avoid eliding all font names, as shown in this screenshot.  I'm not sure why; perhaps it's because the implementation isn't taking the scrollbar into account when setting the menupopup width?

In any case, it probably makes sense to do this, although it doesn't completely solve the problem.  The question then is just whether we should still add the tooltips.  (And whether to set a max-width on the menupopup.)
Attachment #8908446 - Flags: feedback?(nhnt11)
Here's what sizetopopup="none" looks like on Windows.  Same issue there: the menupopup is wider, but not wide enough to avoid eliding a particularly long font name.
Attachment #8908446 - Attachment description: screenshot of elided font names with sizetopopup="none" → screenshot of elided font names with sizetopopup="none" on Mac
(In reply to Myk Melez [:myk] [@mykmelez] from comment #14)
> Created attachment 8908446 [details]
> screenshot of elided font names with sizetopopup="none"
> 
> (In reply to Nihanth Subramanya [:nhnt11] from comment #12)
> 
> > Hmm, so I thought about this more, and here's another idea: if we change the
> > `sizetopopup` attribute value of the menulist to `none` instead of `pref`,
> > the popup's width will not be constrained to the width of the menu
> > (https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Attribute/
> > sizetopopup).
> > 
> > What do you think about doing this, along with a reasonable max-width on the
> > menupopup for the edge case in which the user has a ridiculously-long-named
> > font installed? This might even let us get away with a smaller width value
> > for the menu, since opening the popup will make it immediately obvious what
> > font is selected.
> 
> sizetopopup="none" does increase the width of the menupopup on my Mac,
> although it doesn't do so enough to avoid eliding all font names, as shown
> in this screenshot.  I'm not sure why; perhaps it's because the
> implementation isn't taking the scrollbar into account when setting the
> menupopup width?

Ahh, this is unfortunate, seems like it's because scrollbars are auto-shown by default on MacOS and you've got it set to have them always shown, and the implementation is indeed unaware of this. EDIT: I just saw the Windows screenshot. I don't think there's an auto-hide-scrollbars option at all in Windows, is there? So if I'm not wrong, the longest font name will *always* be cropped on Windows, which is sad, so I don't think we should bother with this.

It annoys me that we don't show tooltips for cropped labels like we do for other overflowed text. Short of fixing this implementation shortcoming, I tinkered for a minute to see if we could use a combination of CSS max-width and text-overflow on the xul:labels in the menupopup to make it work "for free", but I didn't have much luck.

Anyway, I'd like to sleep on this! I'll revisit soon.
(In reply to Nihanth Subramanya [:nhnt11] from comment #16)
> Anyway, I'd like to sleep on this! I'll revisit soon.

Ok, sounds good!  In the meantime, I'll push the patch you r+ed in order to fix the immediate regression.  Then we can contemplate additional enhancements without the pressure of the regression impact.
Pushed by myk@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/686f19f22226
set a wider explicit width on the 'default font' menulist; r=nhnt11
https://hg.mozilla.org/mozilla-central/rev/686f19f22226
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Attachment #8908446 - Flags: feedback?(nhnt11)
Flags: qe-verify+
QA Contact: camelia.badau
I have reproduced this Bug with Nightly 57.0a1 (2017-09-14) on Windows 10, 64 Bit!

The fix is now verified on latest Beta 57.0b7

Build ID 	20171009192146
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
QA Whiteboard: [testday-20171013]
I managed to reproduce this bug using an older version of Nightly (2017-09-14) on Windows 10x64, Ubuntu 16.04x64 and Mac OS X 10.11.
I retested everything using latest Nightly 58 and beta 57.0b8 on the same platforms, but the bug is not reproducing anymore.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: