Closed
Bug 1392532
Opened 7 years ago
Closed 7 years ago
The font size on Linux is too huge, compared to Mac and Windows.
Categories
(Firefox :: Settings UI, defect, P1)
Tracking
()
RESOLVED
FIXED
Firefox 57
People
(Reporter: evanxd, Assigned: rickychien)
References
(Blocks 1 open bug)
Details
(Whiteboard: [photon-preference])
Attachments
(5 files, 1 obsolete file)
Since we use "rem" unit to set the font size in Photon visual refresh work, the actual font size might be too huge when the default system font size is too huge too. The default font size on Ubuntu is 14.6667px, Windows is 12px, and Mac is 11px. The "linux-preferences.png" attachment is the screenshot of Preferences page on Linux (Ubuntu 16.04). The Windows (Windows 10) and Mac (macOS 10.12.5) ones will be updated later. And we could see the font size might be too huge, compared to Windows and Mac. Discussed with Photon Preferences visual designer, Helen in person, we would like to update the the font size for each OS platform to optimize our visual refresh work. We don't have the specific spec yet, we will also need to get feedback from Helen in this bug for the font size info on each OS platform.
Reporter | ||
Comment 1•7 years ago
|
||
Reporter | ||
Comment 2•7 years ago
|
||
Reporter | ||
Updated•7 years ago
|
Summary: The font size on Linux is too huge compare to Mac and Windows. → The font size on Linux is too huge, compared to Mac and Windows.
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Flags: qe-verify-
Whiteboard: [photon-preference][triage] → [photon-preference]
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8899731 [details] Bug 1392532 - Set specific font size with rem unit for each OS platform to optimize Photon visual refresh work since the default system font size values are different on different OS platform. https://reviewboard.mozilla.org/r/171060/#review176654 ::: toolkit/themes/linux/global/in-content/common.css:142 (Diff revision 5) > + > +input, > +.indent, > +.help-button, > +.indent > description { > + font-size: 0.97rem; It looks weird / inconsistent that we're making the text input fields smaller than the surrounding text. Can we stop doing that? Decreasing the text size for .indent looks pretty random too.
Reporter | ||
Comment 11•7 years ago
|
||
Comment on attachment 8899731 [details] Bug 1392532 - Set specific font size with rem unit for each OS platform to optimize Photon visual refresh work since the default system font size values are different on different OS platform. Hi Helen, This is the font size update for each OS platform. Could you review this visual change? You can download the binary from the try[1], and you also check the screenshot of each OS platform. 1. Linux: http://i.imgur.com/YesXwWg.png 2. Windows: http://i.imgur.com/y3ScHDE.png 3. Mac: http://i.imgur.com/gOC0RjX.png Thank you. [1]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea9f3c319d21
Attachment #8899731 -
Flags: ui-review?(hhuang)
Reporter | ||
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8899731 [details] Bug 1392532 - Set specific font size with rem unit for each OS platform to optimize Photon visual refresh work since the default system font size values are different on different OS platform. https://reviewboard.mozilla.org/r/171060/#review176672 ::: toolkit/themes/linux/global/in-content/common.css:142 (Diff revision 5) > + > +input, > +.indent, > +.help-button, > +.indent > description { > + font-size: 0.97rem; Hi Dão, This is a visual design issue. Let's get feeback from our visual designer, Helen.
Reporter | ||
Comment 13•7 years ago
|
||
Hi Helen, What do you think of Comment 10? Thank you.
Flags: needinfo?(hhuang)
Comment 14•7 years ago
|
||
Hi Dão, Since we didn't refresh the components in this version, I reuse the current search field without changing it. So we could consider fixing this once the components are updated. For the text size of the description, It is intended to make it smaller than Body, which could help users to differentiate it with the setting items at a glance.
Comment 15•7 years ago
|
||
Comment on attachment 8899731 [details] Bug 1392532 - Set specific font size with rem unit for each OS platform to optimize Photon visual refresh work since the default system font size values are different on different OS platform. I noticed that the string "Nightly is currently..." is slightly small, it should be as big as body text, and set in semibold font. Please fix it.
Flags: needinfo?(hhuang)
Attachment #8899731 -
Flags: ui-review?(hhuang) → ui-review-
Comment hidden (mozreview-request) |
Reporter | ||
Comment 18•7 years ago
|
||
Comment on attachment 8899731 [details] Bug 1392532 - Set specific font size with rem unit for each OS platform to optimize Photon visual refresh work since the default system font size values are different on different OS platform. Hi Helen, I've updated the patch for the review comments. Could you help review it again? The below is the screenshot of Linux: http://imgur.com/a/yESFv Windows: http://imgur.com/a/gkJ6r Mac: http://imgur.com/a/v3FtI Thank you.
Attachment #8899731 -
Flags: ui-review- → ui-review?(hhuang)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Reporter | ||
Updated•7 years ago
|
Attachment #8899731 -
Flags: review?(jaws)
Reporter | ||
Comment 21•7 years ago
|
||
Hi Jared, Helen already had reviewed the patch previously on Comment 15. And I've updated it for the review comments. (She haven't reviewed it again because she is on PTO today.) Could you help review it to see if we have any technical issue there? Thank you.
Reporter | ||
Comment 22•7 years ago
|
||
(In reply to Helen Huang [:HHuang] from comment #15) > Comment on attachment 8899731 [details] > Bug 1392532 - Set specific font size with rem unit for each OS platform to > optimize Photon visual refresh work since the default system font size > values are different on different OS platform. > > I noticed that the string "Nightly is currently..." is slightly small, it > should be as big as body text, and set in semibold font. Please fix it. As my understand of the comment, the visual change is good to Helen except the font size and weight of the "Nightly is currently..." string.
Comment 23•7 years ago
|
||
mozreview-review |
Comment on attachment 8899731 [details] Bug 1392532 - Set specific font size with rem unit for each OS platform to optimize Photon visual refresh work since the default system font size values are different on different OS platform. https://reviewboard.mozilla.org/r/171060/#review178710 ::: toolkit/themes/linux/global/in-content/common.css:142 (Diff revision 8) > + > +input, > +.indent, > +.help-button, > +.indent > description { > + font-size: 0.97rem; We should remove input, .indent and .indent > description from this rule.
Comment 24•7 years ago
|
||
Comment on attachment 8899731 [details] Bug 1392532 - Set specific font size with rem unit for each OS platform to optimize Photon visual refresh work since the default system font size values are different on different OS platform. They look good to me, thanks!
Attachment #8899731 -
Flags: ui-review?(hhuang) → ui-review+
Comment 25•7 years ago
|
||
Comment on attachment 8899731 [details] Bug 1392532 - Set specific font size with rem unit for each OS platform to optimize Photon visual refresh work since the default system font size values are different on different OS platform. Redirecting to Dao since he's already taken a couple looks at the patch.
Attachment #8899731 -
Flags: review?(jaws) → review?(dao+bmo)
Assignee | ||
Comment 26•7 years ago
|
||
According visual spec, the font size of (2) email address (15px) [1] should be smaller than user name (17px). Evan, can you update the font size for Firefox account page along with your patch? Thanks [1] https://mozilla.invisionapp.com/share/X8BGCX9PD#/screens/249684858
Flags: needinfo?(evan)
Reporter | ||
Comment 27•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #23) > Comment on attachment 8899731 [details] > Bug 1392532 - Set specific font size with rem unit for each OS platform to > optimize Photon visual refresh work since the default system font size > values are different on different OS platform. > > https://reviewboard.mozilla.org/r/171060/#review178710 > > ::: toolkit/themes/linux/global/in-content/common.css:142 > (Diff revision 8) > > + > > +input, > > +.indent, > > +.help-button, > > +.indent > description { > > + font-size: 0.97rem; > > We should remove input, .indent and .indent > description from this rule. Hi Dão, What do you think of Comment 14 from Helen, our visual designer. This style rule is what we want for Photon design.
Flags: needinfo?(evan) → needinfo?(dao+bmo)
Reporter | ||
Comment 28•7 years ago
|
||
(In reply to Ricky Chien [:rickychien] from comment #26) > According visual spec, the font size of (2) email address (15px) [1] should > be smaller than user name (17px). Evan, can you update the font size for > Firefox account page along with your patch? > > Thanks > > > [1] https://mozilla.invisionapp.com/share/X8BGCX9PD#/screens/249684858 Sure, I'll update it.
Comment 29•7 years ago
|
||
Follow up the previous comment, the style of the components will keep the original design. I check the search bars in Firefox, the text of search bar in sidebars and addons page is smaller than body text, for the consistency, I strongly suggest to keep it as it is for now. However, the input field inside of the content area should use the font size for body text, which also follows the original design. For the description, currently we have the guideline for the font usage (http://design.firefox.com/photon/visual/typography.html#styles), and so it will be applied in Firefox not just in Preferences. Dão, hope you can take it into consideration and look forward to having your feedback.
Comment hidden (mozreview-request) |
Reporter | ||
Updated•7 years ago
|
Attachment #8899731 -
Flags: review?(jaws)
Reporter | ||
Updated•7 years ago
|
Attachment #8899731 -
Flags: review?(dao+bmo)
Comment 31•7 years ago
|
||
(In reply to Helen Huang [:HHuang] from comment #29) > Follow up the previous comment, the style of the components will keep the > original design. I check the search bars in Firefox, the text of search bar > in sidebars I believe this is a special case for the sidebar on Mac. We don't do this on Windows and Linux, nor is it the case that all search fields on Mac should have smaller text. > and addons page is smaller than body text, for the consistency, This is because about:addons uses the same stylesheet as preferences, so that's not a useful reference. (In reply to Evan Tseng [:evanxd] from comment #22) > (In reply to Helen Huang [:HHuang] from comment #15) > > Comment on attachment 8899731 [details] > > Bug 1392532 - Set specific font size with rem unit for each OS platform to > > optimize Photon visual refresh work since the default system font size > > values are different on different OS platform. > > > > I noticed that the string "Nightly is currently..." is slightly small, it > > should be as big as body text, and set in semibold font. Please fix it. > > As my understand of the comment, the visual change is good to Helen except > the font size and weight of the "Nightly is currently..." string. This is because that text uses the indent class. As mentioned earlier, we should stop making .indent smaller than the surrounding text.
Flags: needinfo?(dao+bmo)
Comment hidden (mozreview-request) |
Reporter | ||
Comment 33•7 years ago
|
||
Hi Dão, I update the patch for the review comments. Could you take a look? I've removed the `input`, `.indent`, and `.indent > description` from the rule and added the new rules to match our Photon visual design. According to Photon's visual design, we still need a specific style rule for some additional information, like the `These settings are tailored to your computer’s hardware and operating system.` string in Performance groupbox in General pane of Preferences page. See it in the attachment. Same thing for the search inputs on Preferences and about:addons pages. What do you think of our new change? Thank you.
Comment 34•7 years ago
|
||
>
> According to Photon's visual design, we still need a specific style rule for
> some additional information, like the `These settings are tailored to your
> computer’s hardware and operating system.` string in Performance groupbox in
> General pane of Preferences page. See it in the attachment. Same thing for
> the search inputs on Preferences and about:addons pages.
As UX defined, we need to provide more information to describe the function for some settings, and defined it should be smaller than body text, to have some differentiation. But it doesn't mean all the indents should be small, only the sentence is defined as descriptions. I think Evan already helps to fix it.
For the search bar on the top right corner, it's the only one I made the text a bit smaller to align with about:addons; however, this change should not affect the rest of the input fields, all the text of the input field should be as big as body text, except for this one.
Reporter | ||
Updated•7 years ago
|
Flags: needinfo?(dao+bmo)
Comment 35•7 years ago
|
||
(In reply to Helen Huang [:HHuang] from comment #34) > As UX defined, we need to provide more information to describe the function > for some settings, and defined it should be smaller than body text, to have > some differentiation. But it doesn't mean all the indents should be small, > only the sentence is defined as descriptions. I think Evan already helps to > fix it. Okay, so instead of overloading indent, we should have a class specifically for those cases. > For the search bar on the top right corner, it's the only one I made the > text a bit smaller to align with about:addons; however, this change should > not affect the rest of the input fields, all the text of the input field > should be as big as body text, except for this one. Modelling about:preferences after about:addons is nonsensical because it's a circular reference. As mentioned earlier, about:preferences and about:addons share a stylesheet, and the search field in about:addons is smaller only because that stylesheet currently makes all input fields smaller for no good reason that I can see.
Flags: needinfo?(dao+bmo)
Comment 36•7 years ago
|
||
Yes, I agree with you that all the input field should not be smaller. I only ask for decreasing the font size for the "Find in Preferences" search field. There are some reasons that we found the string "Fine in Preferences" will be truncated in some languages in current font size, so we increased the width of the search field. If the text is set as big as body text for now, it needs to get much wider. That might effect the visual hierarchy. Your thoughts?
Flags: needinfo?(dao+bmo)
Comment 37•7 years ago
|
||
(In reply to Helen Huang [:HHuang] from comment #36) > Yes, I agree with you that all the input field should not be smaller. > > I only ask for decreasing the font size for the "Find in Preferences" search > field. There are some reasons that we found the string "Fine in Preferences" > will be truncated in some languages in current font size, so we increased > the width of the search field. If the text is set as big as body text for > now, it needs to get much wider. That might effect the visual hierarchy. > Your thoughts? It might be best to let locales control this like here: http://searchfox.org/mozilla-central/source/browser/locales/en-US/chrome/browser/browser.dtd#904-909 http://searchfox.org/mozilla-central/source/browser/base/content/urlbarBindings.xml#2663
Flags: needinfo?(dao+bmo)
Reporter | ||
Comment 39•7 years ago
|
||
Since I'm on the PTO, Ricky will continue to help on this issue. Thank you very much, Ricky.
Assignee: evan → rchien
Comment 40•7 years ago
|
||
mozreview-review |
Comment on attachment 8899731 [details] Bug 1392532 - Set specific font size with rem unit for each OS platform to optimize Photon visual refresh work since the default system font size values are different on different OS platform. https://reviewboard.mozilla.org/r/171060/#review180718
Attachment #8899731 -
Flags: review?(dao+bmo)
During SV mid-nightly sign offs, this bug was listed among a few others as a P1 bug that should be fixed before pre-beta sign off of "preferences visual refresh (Photon)" feature. Tracked for 57. FYI JGong.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8899731 -
Attachment is obsolete: true
Assignee | ||
Comment 43•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #37) > It might be best to let locales control this like here: > > http://searchfox.org/mozilla-central/source/browser/locales/en-US/chrome/ > browser/browser.dtd#904-909 > > http://searchfox.org/mozilla-central/source/browser/base/content/ > urlbarBindings.xml#2663 Nice trick for addressing l10n issue! Source location has been changed recently, here is the original example with permalink: http://searchfox.org/mozilla-central/rev/999385a5e8c2d360cc37286882508075fc2078bd/browser/locales/en-US/chrome/browser/browser.dtd#910-915 http://searchfox.org/mozilla-central/rev/999385a5e8c2d360cc37286882508075fc2078bd/browser/base/content/urlbarBindings.xml#2676 Dao, I've submitted the updated patch based on your suggestions, please review it kindly! * Removed specific font size for search input (all font-size of textbox in preferences are using the same size now) * Default width (English) of search input is 230px, this width is dynamic and will be changed depending on language. (Spec will update soon)
Comment hidden (mozreview-request) |
Comment 45•7 years ago
|
||
mozreview-review |
Comment on attachment 8904100 [details] Bug 1392532 - Polish Preferences font size depending on platforms https://reviewboard.mozilla.org/r/175868/#review181056 ::: browser/locales/en-US/chrome/browser/preferences/preferences.dtd:19 (Diff revision 2) > <!ENTITY prefWinMinSize.styleGNOME "width: 45.5em; min-height: 40.5em;"> > > +<!-- LOCALIZATION NOTE: (searchField.width): This is used to determine the width > + of the search field in about:preferences, in order to make entire placeholder > + string visible --> > +<!ENTITY searchField.width "230px"> Please convert this to an em value ::: toolkit/themes/linux/global/in-content/common.css:140 (Diff revision 2) > + font-size: 1.27rem; > +} > + > +.tip-caption, > +.help-button { > + font-size: 0.97rem; I don't think we should go below 1rem. Can you remove this rule (Linux only)?
Assignee | ||
Comment 46•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8904100 [details] Bug 1392532 - Polish Preferences font size depending on platforms https://reviewboard.mozilla.org/r/175868/#review181056 > Please convert this to an em value Ah, this is a new l10n property so it doesn't make any sense to revert it back.
Comment 47•7 years ago
|
||
(In reply to Ricky Chien [:rickychien] from comment #46) > Comment on attachment 8904100 [details] > Bug 1392532 - Polish Preferences font size depending on platforms > > https://reviewboard.mozilla.org/r/175868/#review181056 > > > Please convert this to an em value > > Ah, this is a new l10n property so it doesn't make any sense to revert it > back. I don't understand what this means. Dimensions that depend on the text size should always be in em.
Assignee | ||
Comment 48•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8904100 [details] Bug 1392532 - Polish Preferences font size depending on platforms https://reviewboard.mozilla.org/r/175868/#review181056 > Ah, this is a new l10n property so it doesn't make any sense to revert it back. Sorry, I misread the sentence. em value has been updated. > I don't think we should go below 1rem. Can you remove this rule (Linux only)? I'm going to set it to 1rem instead of going below 1rem. According to visual design spec, font-size of .tip-caption & .help-button should be a slightly smaller than general page font size (it's currently set to 1.11 rem at line 123 within this file).
Comment hidden (mozreview-request) |
Assignee | ||
Comment 50•7 years ago
|
||
BTW, I'm curious about why `em` value is the best unit to fit our need? What I can see the difference after using em value is that the actual width of search field went to different value on different system. Ubuntu is 252px but macOS is 231px. It causes a different look and feel which isn't what we want. Or doesn't any solution we can solve this issue?
Comment hidden (mozreview-request) |
Assignee | ||
Comment 52•7 years ago
|
||
Patch has updated again and finally I figured out a way to set `em` value for all platforms with using #ifdef. The current patch has been tested under all platforms and looks great.
Comment 53•7 years ago
|
||
(In reply to Ricky Chien [:rickychien] from comment #50) > BTW, I'm curious about why `em` value is the best unit to fit our need? What > I can see the difference after using em value is that the actual width of > search field went to different value on different system. Ubuntu is 252px > but macOS is 231px. This should be expected since the text sizes are different. After all we want the placeholder to be visible and the user to type comfortably in the text field, both of which depends on the text size. (In reply to Ricky Chien [:rickychien] from comment #52) > Patch has updated again and finally I figured out a way to set `em` value > for all platforms with using #ifdef. Please revert this. :/ Localizers shouldn't have to worry about this; it's not even clear what platform differences they should comply with here in the first place.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 55•7 years ago
|
||
Sure! Those platform specific `em` values have been reverted. IIRC, we're going to accept different search field width according to platforms since it's reasonable to expect that the width should be dynamic and adjusted corresponding to text size.
Comment hidden (mozreview-request) |
Comment 57•7 years ago
|
||
mozreview-review |
Comment on attachment 8904100 [details] Bug 1392532 - Polish Preferences font size depending on platforms https://reviewboard.mozilla.org/r/175868/#review181182 Thanks!
Attachment #8904100 -
Flags: review?(dao+bmo) → review+
Comment 58•7 years ago
|
||
Pushed by rchien@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9aa2e9106aa3 Polish Preferences font size depending on platforms r=dao
![]() |
||
Comment 59•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9aa2e9106aa3
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Updated•7 years ago
|
Comment 60•7 years ago
|
||
Screenshots: https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=973e8b890a62aee4b3170558ac3b608928162ef6&newProject=mozilla-central&newRev=978d2539a8d1a49e9f9705204f3918772b337547&filter=pref
Comment 61•7 years ago
|
||
mozreview-review |
Comment on attachment 8904100 [details] Bug 1392532 - Polish Preferences font size depending on platforms https://reviewboard.mozilla.org/r/175868/#review184354 ::: toolkit/themes/osx/global/in-content/common.css:132 (Diff revision 6) > xul|textbox[type="search"]:not([searchbutton]) > .textbox-input-box > .textbox-search-sign { > list-style-image: url(chrome://global/skin/icons/search-textbox.svg); > margin-inline-end: 5px; > } > > +html|html *, This selector has high specificity and regresses all font-size rules that use a simple class selector (for example). One such regression is the heading text in about:privatebrowsing. I'm going to file a bug, but in the meantime: Dao, do you think we should revert this, or reduce the specificity of this selector somehow? I haven't yet investigated but I'm afraid that it'll be hard to track down all the regressions, though I'm hoping this won't be the case...
Comment 63•7 years ago
|
||
(In reply to Nihanth Subramanya [:nhnt11] from comment #61) > Comment on attachment 8904100 [details] > Bug 1392532 - Polish Preferences font size depending on platforms > > https://reviewboard.mozilla.org/r/175868/#review184354 > > ::: toolkit/themes/osx/global/in-content/common.css:132 > (Diff revision 6) > > xul|textbox[type="search"]:not([searchbutton]) > .textbox-input-box > .textbox-search-sign { > > list-style-image: url(chrome://global/skin/icons/search-textbox.svg); > > margin-inline-end: 5px; > > } > > > > +html|html *, > > This selector has high specificity and regresses all font-size rules that > use a simple class selector (for example). One such regression is the > heading text in about:privatebrowsing. See bug 1397325 comment 12.
Flags: needinfo?(dao+bmo)
Comment 64•7 years ago
|
||
(In reply to Ritu Kothari (:ritu) from comment #41) > During SV mid-nightly sign offs, this bug was listed among a few others as a > P1 bug that should be fixed before pre-beta sign off of "preferences visual > refresh (Photon)" feature. Tracked for 57. > > FYI JGong. The bug has been fixed.
Flags: needinfo?(jgong)
You need to log in
before you can comment on or make changes to this bug.
Description
•