Closed Bug 1194055 Opened 10 years ago Closed 10 years ago

Size of <input> elements has changed in Firefox 40

Categories

(Core :: Layout: Form Controls, defect)

Product:
Core
Component:
Layout: Form Controls
Version:
40 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla43
Tracking Flags:
Tracking Status
firefox40 + fixed
firefox41 + fixed
firefox42 + fixed
firefox43 + fixed
firefox-esr31 --- unaffected
firefox-esr38 --- unaffected
relnote-firefox --- 40+

People

(Reporter: birtles, Assigned: jfkthame)

References

(
URL
)

Details

(4 keywords)

Attachments

(7 files, 2 obsolete files)

Bug 1194055 input.html
496 bytes, text/html
Details
Screenshot comparison between Edge, Firefox39 and Nightly43.0a01 on Windows10
59.56 KB, image/png
Details
Screenshot comparison between Ie11, Firefox39 and Nightly43.0a01 on Windows7
128.36 KB, image/png
Details
GetSysFontInfo() should return the old UI font, "MS UI Gothic" for -moz-field only on Japanese Windows due to the font size of the new system font is much bigger than old one
1.91 KB, patch
jtd
: review-
Details | Diff | Splinter Review
Workaround for excessively large xAvgCharWidth in Meiryo -- use width of 'zero' instead
1.42 KB, patch
jtd
: review-
m_kato
: feedback-
Details | Diff | Splinter Review
screenshot of <input> and <input size="40">
10.10 KB, image/png
Details
GetSysFontInfo should return MS Shell Dlg 2 for eFont_Field and eFont_List on Windows
2.84 KB, patch
masayuki
: review+
Sylvestre
: approval-mozilla-aurora+
Sylvestre
: approval-mozilla-beta+
Sylvestre
: approval-mozilla-release+
Details | Diff | Splinter Review
We're seeing reports on twitter in Japan of layouts breaking in Firefox 40 due to the sizing of <input type="text"> and <textarea> elements. One report suggests the default font might have changed.[1] For the test URL the following difference is reported: ESR38: https://pbs.twimg.com/media/CMQyxHKUEAA2CBm.png:large 40: https://pbs.twimg.com/media/CMQy27dU8AAtJ-a.png:large [1] https://twitter.com/gooey_archer/status/631658186454401025
Summary: Default size of <input> elements has changed in Firefox 40 → Size of <input> elements has changed in Firefox 40
Is this only in Japanese and Windows environment?
(In reply to Kohei Yoshino [:kohei] from comment #1) > Is this only in Japanese and Windows environment? Not sure. (including whether this is intentional change or not)
Keywords: dev-doc-needed, site-compat
(In reply to Kohei Yoshino [:kohei] from comment #1) > Is this only in Japanese and Windows environment? Yes. This will force the default to be sans-serif. Curiously, the lang of the page here is set to "en".
Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a99ba2338d04&tochange=cf2fb09e09b6 Regressed by: cf2fb09e09b6 Jonathan Kew — Bug 1123654 - Replace use of [deprecated] GetStockObject(DEFAULT_GUI_FONT) with newer API; results in use of Tahoma in place of Microsoft Sans Serif in various contexts. r=jmathies
Blocks: 1123654
Keywords: regressionwindow-wanted
[Tracking Requested - why for this release]: There are a lot of tweets complaining about this issue. If the change isn't intentional, we should fix this for restoring the web.
status-firefox40: --- → affected
tracking-firefox40: --- → ?
The fact that the default font for these elements changed is expected; we were using a deprecated API and getting an old default. I note that viewing the testcase from the URL in IE 11 on Win8.1 gives me a result that's closer to our "new" behavior than our "old". In general, I'm seeing substantial differences between browsers/platforms with the testcase, illustrating the fact that relying on a specific default size of <input> elements is extremely fragile. Authors who want a specific size should be explicitly specifying it. Is there some reason this is particularly problematic in Japan? (What was the old font being used, what is the new font, and -- aside from the effect on size -- is the new font better/worse than the old one? How does it compare to what IE uses?)
Attached file Bug 1194055 input.html (obsolete) —
AFAIK, default font is attached
Attached file Bug 1194055 input.html —
Attachment #8647361 - Attachment is obsolete: true
Wow, that's a considerably greater difference than I see on an en-us system. John, can you look into what's going on here? Do the Japanese system fonts really have such extreme differences in metrics, or are we somehow using the wrong values to compute the <input> size?
Flags: needinfo?(jdaggett)
I'm not sure how much effect it has, but "MS Shell Dlg 2" is the wrong font to use on Vista+. It only ever maps to Tahoma, while on Vista+ the system UI font is Segoe UI. It's possible it's hitting some character fallback.
I think that the main problem is, the old Japanese UI font "MS UI Gothic" and new Japanese UI font "Meiryo UI" have very different size and glyph. A character size of Meiryo UI is too large. That must cause breaking a lot of web sites' layout. Edge still uses "MS UI Gothic" for the default style of <input> according to the testcase. So, we should not use "true" system font on Win8 or later.
Perhaps, the simplest hack for release build is, adding font-family rules with lang selector. I guess that similar problem occurs in Chinese and Korean locale.
MS Shell Dlg2 is mapped to Tahoma. But this also has font link. In HKLM\SOFTWARE\Microsoft\Windows NT\CurrentVersion\FontList\SystemLink MSGOTHIC.TTC,MS UI Gothic MINGLIU.TTC,PMingLiU SIMSUN.TTC,SimSun GULIM.TTC,Gulim YUGOTHM.TTC,Yu Gothic UI MSJH.TTC,Microsoft JhengHei UI MSYH.TTC,Microsoft YaHei UI MALGUN.TTF,Malgun Gothic SEGUISYM.TTF,Segoe UI Symbol
It's not only happening on Windows. FYI Firefox for Android (41 Beta) has "wide" default width, too. http://p.twpl.jp/show/orig/2ABh8 Why not simply add "max-width" to the default style? Is there any reason not to do so?
> Why not simply add "max-width" to the default style? Specifying such rule may break the web...
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #17) > Specifying such rule may break the web... It "may", but is there a realistic case ?
We cannot say "overflow: visible; always causes unexpected result".
(In reply to Makoto Kato [:m_kato] from comment #15) > MS Shell Dlg2 is mapped to Tahoma. > > But this also has font link. > > In HKLM\SOFTWARE\Microsoft\Windows NT\CurrentVersion\FontList\SystemLink > > MSGOTHIC.TTC,MS UI Gothic > MINGLIU.TTC,PMingLiU > SIMSUN.TTC,SimSun > GULIM.TTC,Gulim > YUGOTHM.TTC,Yu Gothic UI > MSJH.TTC,Microsoft JhengHei UI > MSYH.TTC,Microsoft YaHei UI > MALGUN.TTF,Malgun Gothic > SEGUISYM.TTF,Segoe UI Symbol Hmm, on my Win7-Ja, FontLink of Tahoma is: MSGOTHIC.TTC,MS UI Gothic MINGLIU.TTC,PMingLiU SIMSUN.TTC,SimSun GULIM.TTC,Gulim So, not mapping to Meiryo. On the other hand, Segoe UI is: TAHOMA.TTF MEIRYO.TTC,Meiryo,128,85 MEIRYO.TTC,Meiryo MSGOTHIC.TTC,MS UI Gothic MSJH.TTF,128,96 MSJH.TTF MSYH.TTF,128,96 MSYH.TTF MALGUN.TTF,128,96 MALGUN.TTF MINGLIU.TTC,PMingLiU SIMSUN.TTC,SimSun GULIM.TTC,Gulim but FontSubstitues of MS Shell Dlg 2 is "Tahoma". (I can reproduce this bug on the environment, of course)
I don't see similar symptom on Win7-zh_CN nor Win7-ko. I think that only Japanese Windows 7 has the big difference. And I realized that even if I specify Segoe UI or Tahoma with font-family, the result is different from directly specifying MS Shell Dlg 2. I guess that the system font "MS Shell Dlg 2" returns bigger font size??
Oh, input's font is -moz-Field: https://hg.mozilla.org/mozilla-central/annotate/4e883591bb5d/layout/style/forms.css#l63 -moz-Field is mapped to LookAndFeel::eFont_Field: http://mxr.mozilla.org/mozilla-central/source/layout/style/nsRuleNode.cpp?rev=443625267e00#3333 And GetSysFontInfo() in windows/nsLookAndFeel.cpp doesn't handle this. So, the result is the result of SPI_GETNONCLIENTMETRICS? Field case was dropped by the patch for bug 1123654. https://bugzilla.mozilla.org/attachment.cgi?id=8553000&action=diff
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #24) > Created attachment 8647901 [details] [diff] [review] > GetSysFontInfo() should return the old UI font, "MS UI Gothic" for > -moz-field only on Japanese Windows due to the font size of the new system > font is much bigger than old one > > I think that we can back it out only for font: -moz-Field; on Japanese > Windows. This should be the safest patch. I'm not a big fan of this sort of thing. Random changes in fonts is not a good thing but default fonts do change and I don't think hacks like this are such a good thing. Authors really should be designing things in such a way that they are not dependent on defaults never changing.
Flags: needinfo?(jdaggett)
(In reply to John Daggett (:jtd) from comment #25) > (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #24) > > Created attachment 8647901 [details] [diff] [review] > > GetSysFontInfo() should return the old UI font, "MS UI Gothic" for > > -moz-field only on Japanese Windows due to the font size of the new system > > font is much bigger than old one > > > > I think that we can back it out only for font: -moz-Field; on Japanese > > Windows. This should be the safest patch. > > I'm not a big fan of this sort of thing. Random changes in fonts is not a > good thing but default fonts do change and I don't think hacks like this are > such a good thing. Authors really should be designing things in such a way > that they are not dependent on defaults never changing. I agree with that it's a web designers' responsibility. However, not users'. The actual victims are our users. So, such change may make damage to our marketing share. And also, only the Win7 case is too big change (MS UI Gothic vs. Meiryo). That may break the layout even if web sites are designed as no so tight (the width becomes almost x2). Additionally, IE/Edge still users MS UI Gothic for the default font of <input>. So, the big difference isn't good thing for compatibility between browsers. (On the other hand, I'm not sure the Google Chrome's behavior. In the <input> widths of the tescase, Meiryo and Meiryo UI reaches same width, so, they use different rule to compute the width of <input>.) John-san, how do you think about the compatibility issue? I think that if the CSS 3's new system fonts gets better fonts, we shouldn't change the width of <input size="n"> as far as possible. For now, I'd like strongly to suggest the partial backout.
Oops, forgot to ni?.
Flags: needinfo?(jdaggett)
I also think that it might be the best approach to make the default <input> width and its size attribute computed with the legacy system font. Then, only the web sites which don't specify the style explicitly with CSS become safe from the default font changes. However, it's risky for uplift.
I actually think we should back out the changes for bug 1123654, reopen it and rework the logic there. In particular, I think the default widget font should remain what it was before the change made on bug 1123654. The one thing I think this patch highlights is our inability to catch problems with changes made affecting non-English locales. I think we should consider having OS instances from other locales running tests so that we catch problems earlier.
Flags: needinfo?(jdaggett)
(In reply to John Daggett (:jtd) from comment #29) > I actually think we should back out the changes for bug 1123654, reopen it > and rework the logic there. In particular, I think the default widget font > should remain what it was before the change made on bug 1123654. I agree this approach for Beta, Aurora and Nightly. However, isn't it risky for release? (the other fonts will be changed too) > The one thing I think this patch highlights is our inability to catch > problems with changes made affecting non-English locales. I think we should > consider having OS instances from other locales running tests so that we > catch problems earlier. Yeah, but it must be difficult actually :-( jfkthame: Could you back it out from Beta, Aurora and Nightly?
Flags: needinfo?(jfkthame)
Flags: needinfo?(jdaggett)
I'm not sure who is the best person to ask this. This bug causes breaking layout of Japanese website especially on Windows 7 (see the screenshot). Therefore, I think that we should fix this bug ASAP even in release build. I think that the regressed patch should be backed out from Beta, Aurora and Nightly. But it sounds risky to me for Release. I'm not testing the patch which backouts only on Japanese Windows and only the style of |font: -moz-Field;| which is used only by <input> in forms.css. I'd like to know how release drivers think this bug.
Flags: needinfo?(sledru)
Bug 1123654 landed 4 months ago. I am surprised that we are only noticing this bug now?! I am afraid that doing a backout might have other side effects... (not mentioning that it might be hard to do)
Flags: needinfo?(sledru)
(In reply to Sylvestre Ledru [:sylvestre] from comment #32) > Bug 1123654 landed 4 months ago. I am surprised that we are only noticing > this bug now?! Yes. The only big difference is reproduced only on Win7-Ja. So, there are not so many testers... (And if websites specify the width of <input> with CSS, the difference isn't problem) > I am afraid that doing a backout might have other side effects... (not > mentioning that it might be hard to do) Yeah, but I see several tweets about this bug even only in Army of Awesome. So, I believe that this bug makes not a few users feel inconvenience. Sigh...
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #30) > jfkthame: > > Could you back it out from Beta, Aurora and Nightly? I'm not sure whether that's the right way forward here. While it would fix this issue, it would obviously regress the original issue with the fonts used in en-US etc. (And bug 1123654#c3 notes that we were also having a problem in Thai, for example.) Clearly, the result we're getting in Japanese is a problem; but it's not clear to me whether the best way to address that is to revert everything and start over, or to apply a Japanese-specific fix. (We should also investigate the results on Chinese and Korean systems, I guess, as they may be the likeliest locales to show similar issues.) (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #33) > Yes. The only big difference is reproduced only on Win7-Ja. So, there are > not so many testers... (And if websites specify the width of <input> with > CSS, the difference isn't problem) If sites specify the width (e.g. in px), do we end up with significantly fewer characters visible within the <input> element, or does roughly the same amount of text fit as previously? I.e. given an element like <input style="width:100px">, how many (Japanese) characters fit using Firefox39? How many fit with Firefox40?
Flags: needinfo?(jfkthame) → needinfo?(masayuki)
(In reply to Jonathan Kew (:jfkthame) from comment #34) > (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #30) > > jfkthame: > > > > Could you back it out from Beta, Aurora and Nightly? > > I'm not sure whether that's the right way forward here. While it would fix > this issue, it would obviously regress the original issue with the fonts > used in en-US etc. (And bug 1123654#c3 notes that we were also having a > problem in Thai, for example.) > > Clearly, the result we're getting in Japanese is a problem; but it's not > clear to me whether the best way to address that is to revert everything and > start over, or to apply a Japanese-specific fix. (We should also investigate > the results on Chinese and Korean systems, I guess, as they may be the > likeliest locales to show similar issues.) So, my patch only backouts for font-family: -moz-Field; on Windows-Ja. But John-san doesn't like such approach :-( FYI: I tested on Chinese and Korean Windows 7 and 8.1, but the width isn't so wide. Only the Win7-Ja case is too bad. > (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #33) > > > Yes. The only big difference is reproduced only on Win7-Ja. So, there are > > not so many testers... (And if websites specify the width of <input> with > > CSS, the difference isn't problem) > > If sites specify the width (e.g. in px), do we end up with significantly > fewer characters visible within the <input> element, or does roughly the > same amount of text fit as previously? > > I.e. given an element like <input style="width:100px">, how many (Japanese) > characters fit using Firefox39? How many fit with Firefox40? According to this example: http://jsfiddle.net/d_toybox/wjxxLLp5/ Meiryo's character width is wider than the others. The others can be about 10 characters of "あ". But Meiryo can be only 8 characters of "あ". So, the change make <input> tighter on Win7-ja.
Flags: needinfo?(masayuki)
Comment on attachment 8647901 [details] [diff] [review] GetSysFontInfo() should return the old UI font, "MS UI Gothic" for -moz-field only on Japanese Windows due to the font size of the new system font is much bigger than old one Until we find the best approach, I'd like to suggest that we should back it out partially only for Windows-Ja.
Attachment #8647901 - Flags: review?(jfkthame)
I looked into this a bit more, and the root of the problem seems to be that Meiryo has an unexpectedly large value for xAvgCharWidth (in the OS/2 table): 1958 units, or 0.956em. Compare that with the value in Meiryo UI: 1099 units, or 0.537em. This leads to the behavior we're seeing here because the intrinsic width of an <input> element is computed from its |size| attribute if present (defaulting to 20), multiplied by the aveCharWidth value from the font metrics, which comes from OS/2:xAvgCharWidth. Interestingly, if you specify the width via CSS as "width: 20ch", which might be expected to be similar to using the attribute "size=20", the result is much more reasonable. This is because we base the 'ch' unit on the zeroOrAveCharWidth field in our font metrics, which is derived from the width of the 'zero' glyph if present in the font, falling back to aveCharWidth otherwise. I suspect it's largely a historical accident that the HTML 'size' attribute and the CSS 'ch' unit work so differently. Checking the HTML spec[1] it just says that "[t]he size attribute gives the number of characters that, in a visual rendering, the user agent is to allow the user to see while editing the element's value", which only makes sense assuming a fixed-pitch font, and doesn't give any guidance as to what the UA should do if the element is using a proportional font. Experimentation shows that UAs do not generally size <input> elements such that they show 20 fullwidth Kanji by default; the number visible is quite variable (sometimes not much more than 10). Which makes me wonder how meaningful the HTML spec's statement really is. The CSS spec[2] is somewhat more helpful, specifying that the 'ch' unit is "[e]qual to the used advance measure of the "0" (ZERO, U+0030) glyph found in the font used to render it". (That's indeed what we use, except that if there's no ZERO in the font, it looks like we'll use its aveCharWidth rather than getting the width of zero from a fallback font. Which in practice should rarely be an issue.) I'm wondering whether we could get away with harmonizing 'size' and 'ch', by making the 'size' computation rely on the same unit of character width as CSS's 'ch'. ISTM this would provide authors with a much more predictable, understandable, and consistent behavior. Of course, there's the possibility this would break the web...... but given how much variability there is in 'size' already, depending on the font the browser happens to be using, perhaps the web is robust enough to allow this? I ran a try build https://treeherder.mozilla.org/#/jobs?repo=try&revision=b892c59048f3 where 'size' is computed using the zeroOrAveCharWidth value, like CSS 'ch'. AFAICT, this should make things considerably better for the Japanese issues; Masayuki, WDYT? John? [1] https://html.spec.whatwg.org/multipage/forms.html#attr-input-size [2] http://www.w3.org/TR/css3-values/#font-relative-lengths
Flags: needinfo?(masayuki)
Here's an alternative approach: instead of reverting to the deprecated GetStockObject, this directly targets the bad avgCharWidth in Meiryo, and should make the impact of the font change on layout much less dramatic. If we want a fix that could be uplifted to beta, I think this might be the tidiest/safest way to go.
Attachment #8648403 - Flags: review?(jdaggett)
Attachment #8648403 - Flags: review?(masayuki)
:masayuki, how about the alternative suggestion above -- would this be an acceptable solution for Japanese? (Does the patch work as expected on localized Japanese systems? Try build is in progress...)
Thank you for your work. If I could check your patch tomorrow, I'd do that. But I'm not sure if I have much time tomorrow because I need to go out.
Comment on attachment 8648403 [details] [diff] [review] Workaround for excessively large xAvgCharWidth in Meiryo -- use width of 'zero' instead I feel this behavior is reasonable if I don't mind the compatibility between browsers (Although, I've not tested on Win7, but at least working well on Win10). The new width makes our behavior of <input style="font-style: Meiryo;"> similar to (a little bit wider than) "Meiryo UI" case. This gives us similar result of Chrome, however, completely different from the result of IE/Edge (Our old Meiryo result is same as IE/Edge). Although, the new behavior could be better result than current behavior, however, I really worry about breaking the compatibility with IE/Edge. See this market share of browsers in Japan: http://gs.statcounter.com/#browser-JP-monthly-201407-201507 In Japan, IE has about 40% market share and Chrome has about 30%. So, in Japan, IE is still the most popular web browser. Additionally, this market share should be counted in public web pages. So, I believe that IE's market share in enterprise must be greater than 40% because its very long life cycle is much better than the other browsers. So, we still should give priority keeping compatibility with IE rather than Chrome in Japan. Therefore, I feel that this approach is a little bit risky for the other cases which use aveCharWidth of our font metrics. (My approach only affects for font: -moz-Field; which is used for only for <input>, restricted to Japanese Windows and we give <input> the traditional UI font again. Therefore, I still believe that my approach is safer than this.) With these reasons, I worry about that we take this approach for the other cases which use aveCharWidth. (I just canceling r?, not r-, due to I'm not 100% sure if this is really risky.) Any ideas, Makoto-san?
Flags: needinfo?(masayuki)
Attachment #8648403 - Flags: review?(masayuki) → feedback?(m_kato)
And also, bug 1123654 shouldn't be reproduced with Japanese Windows because MS UI Gothic's "…" has enough width and it's not used in Japanese Firefox due to the glyph problem (bug 403484).
I prefer to use the "zero" glyph width because xAvgCharWidth is unreliable. For OS/2 table version 2 or earlier, xAvgCharWidth was the average of the width of 26 lowercase letters (that is, "half-width" letters in East Asian contexts): https://www.microsoft.com/typography/otspec/os2ver2.htm#acw Starting from version 3, xAvgCharWidth is supposed to be the average of all non-zero width glyphs in the font that include all ideographs (that is, "full-width" characters): https://www.microsoft.com/typography/otspec/os2ver3.htm#acw For CJK fonts, the version 3+ xAvgCharWidth will be twice larger than version 2 xAvgCharWidth because overwhelming majority of glyphs is full-width. Not all CJK fonts follow the OS/2 ver.3+ xAvgCharWidth change because some apps don't consider the difference between ver.2 and ver.3. For example, Meiryo UI uses OS/2 table version 2 to workaround the issue. Yu Gothic UI uses approximately the width of half-width character glyphs despite that the OS/2 table version is 4.
Comment on attachment 8648403 [details] [diff] [review] Workaround for excessively large xAvgCharWidth in Meiryo -- use width of 'zero' instead As long as I test my Japanese Windows box, root cause is that default font of editbox (and widgets) is changed to MS Gothic to Meiryo by Firefox 40 unfortunately. Should we accept to change default font?
According to this screenshot of the tryserver build, each width/height is: MS UI Gothic, no size: 135px (100%), size=40: 255px (100%), height: 20px (100%) Meiryo, no size: 193px (142%), size=40: 358px (140%), height: 27px (135%) Meiryo UI, no size: 170px (126%), size=40: 314px (123%), height: 24px (120%) Yu Gothic UI, no size: 162px (120%), size=40: 305px (120%), height: 25px (125%) Even with the tryserver build, Meiryo's with becomes x1.4, the height is x1.35. I feel this may have big impact for some pages (indeed, the number of such pages must be less than now). I don't disagree with changing to restrict the aveCharWidth of Meiryo. However, I worry about that the font change keeps causing broken layout in some web pages. I still don't have a reason we should use new UI font for <input> on *Japanese* Windows since the regression cause wasn't problem on Japanese Windows.
Comment on attachment 8648403 [details] [diff] [review] Workaround for excessively large xAvgCharWidth in Meiryo -- use width of 'zero' instead Review of attachment 8648403 [details] [diff] [review]: ----------------------------------------------------------------- I don't think that changing char width of font is good. Original problem is that default font of editbox is changed.
Attachment #8648403 - Flags: feedback?(m_kato) → feedback-
- Windows 7 JA Firefox 39 ... default is MS UI(P) Gothic. Same as IE and Chrome Firefox 40 ... default is Meiryo. It is different of other browers - Windows 10 JA Firefox 39 ... default is MS UI(P) Gothic. Same as Chrome, but IE11 and Edge is Meiryo UI. Firefox 40 ... default is Meiryo UI. Same as IE and Edge. But Chrome is still MS UI(P) Gothic. So this is default font issue, not width. If default font is changed, width will be changed.
ah, test is invalid When using Edge and IE11 on Windows 10, it is still MS UI/P Gothic.
When I talk with John-san about this last week, he point out that SPI_GETNONCLIENTMETRICS may not correct for editbox (LookAndFeel::eFont_Field). I think that eFont_Field should not use SPI_GETNONCLIENTMETRICS's message font.
Attachment #8647901 - Flags: review-
Comment on attachment 8648403 [details] [diff] [review] Workaround for excessively large xAvgCharWidth in Meiryo -- use width of 'zero' instead Review of attachment 8648403 [details] [diff] [review]: ----------------------------------------------------------------- Yeah, I don't think we should do this, as it will affect other things when Meiryo is included in a fontlist.
Attachment #8648403 - Flags: review?(jdaggett) → review-
(In reply to Makoto Kato [:m_kato] from comment #53) > When I talk with John-san about this last week, he point out that > SPI_GETNONCLIENTMETRICS may not correct for editbox > (LookAndFeel::eFont_Field). > > I think that eFont_Field should not use SPI_GETNONCLIENTMETRICS's message > font. Do you think that we should use legacy UI font for eFont_Field in any locales (i.e., backing the patch out for eFont_Field case)? If so, I support the idea.
Flags: needinfo?(m_kato)
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #55) > (In reply to Makoto Kato [:m_kato] from comment #53) > > When I talk with John-san about this last week, he point out that > > SPI_GETNONCLIENTMETRICS may not correct for editbox > > (LookAndFeel::eFont_Field). > > > > I think that eFont_Field should not use SPI_GETNONCLIENTMETRICS's message > > font. > > Do you think that we should use legacy UI font for eFont_Field in any > locales (i.e., backing the patch out for eFont_Field case)? If so, I support > the idea. We should use same font as other browsers, specially IE/Edge. At least, IE, Edge and Chrome still use MS Gothic font that is as same as OS control. So we should select OS control font. Also this has same issue for eFont_List too. Bug 1123654 sets MS Shell Dlg2 as button font (button font isn't changed even if Firefox 40 on Windows 7 and 10), so we should set same for text field and listbox too. I cannot make sense why bug 1123654 doesn't set same font for text field and button and etc.
Flags: needinfo?(m_kato)
Also, even if Windows 7 English version, default font is changed. Before 40 ... MS Shell Dlg After 40 ... Segoe UI IE on Windows 7 EN uses MS Shell Dlg...
I'd be pretty reluctant to add locale-specific or Windows version-specific special case code here (although if we want a low-risk fix for beta, that might be the safest way to do it). In general, though, we should be integrating with the available Windows APIs in a generic way. I'd also be reluctant to revert to the GetStockObject API, which MSDN says is "not recommended" for getting fonts; it points us towards SPI_GETNONCLIENTMETRICS and/or the MS Shell Dlg 2 virtual font name instead. However, the implementation from bug 1123654 may indeed need to be improved... Specifically, it sounds to me like perhaps we should add eFont_Field and eFont_List to the cases where GetSysFontInfo returns the virtual "MS Shell Dlg 2" font name; would that address the concern here? I've started a try build so we can see how this goes: https://treeherder.mozilla.org/#/jobs?repo=try&revision=505c0945e6ed. It's likely there will still be some difference from the metrics in older FF versions. However, I don't think that should be a critical issue. Authors need to accept that default font metrics, control sizes, etc do vary between systems, and either use explicit styling to achieve the exact effect they want, or layouts that are sufficiently robust to allow for reasonable variations.
Does this help to address the issue on Japanese systems?
Attachment #8648924 - Flags: feedback?(masayuki)
Attachment #8648924 - Flags: feedback?(m_kato)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Comment on attachment 8648924 [details] [diff] [review] GetSysFontInfo should return MS Shell Dlg 2 for eFont_Field and eFont_List on Windows Maybe, you need adjust fuzzy parameter of reftest for 363858-5a.html and 363858-6a.html.
Attachment #8648924 - Flags: feedback?(m_kato) → feedback+
Tested Windows 10 JA and Windows 7 JA. It works.
Comment on attachment 8648924 [details] [diff] [review] GetSysFontInfo should return MS Shell Dlg 2 for eFont_Field and eFont_List on Windows I don't see any problem on Win10-Ja and Win7-Ja (except the width change of <input>). I'll check this on other platforms too before judging the feedback result.
Comment on attachment 8648924 [details] [diff] [review] GetSysFontInfo should return MS Shell Dlg 2 for eFont_Field and eFont_List on Windows Hmm, the build crashes on WinXP when I open context menu or typing Japanese character in the <input>. I guess that following change causes this: > - memcpy(name, ptrLogFont->lfFaceName, LF_FACESIZE*sizeof(char16_t)); > - aFontName = name; > + aFontName = ptrLogFont->lfFaceName;
Attachment #8648924 - Flags: feedback?(masayuki) → feedback-
> the build crashes on WinXP when I open context menu or typing Japanese character in the <input>. Hmm, I reproduced this on Win7-en-US too.
As far as I tested on each locale (en-US, ko-KR, zh-CN, zh-TW, zh-HK (only Win8.1) and ja-JP) on both Win8.1 and Win7, I don't see any problems except the crash (underline of composition string isn't hidden in any platforms). I'd like to check WinXP-Ja after the crash is fixed. I don't have WinVista environment and non-ja-JP locale's WinXP, Win10.
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #63) > Comment on attachment 8648924 [details] [diff] [review] > GetSysFontInfo should return MS Shell Dlg 2 for eFont_Field and eFont_List > on Windows > > Hmm, the build crashes on WinXP when I open context menu or typing Japanese > character in the <input>. I guess that following change causes this: > > > - memcpy(name, ptrLogFont->lfFaceName, LF_FACESIZE*sizeof(char16_t)); > > - aFontName = name; > > + aFontName = ptrLogFont->lfFaceName; Drat, sorry about that. I thought that was harmless cleanup, but apparently not.... I'll push a build with it reverted. (I wonder why it didn't crash on tryserver?)
Here's the simplified patch; hoping this no longer causes unexpected crashes. I also adjusted the fuzz annotation for 363858-5a and 363858-6a (that's not in the try push, so we'll still see reftest orange there).
Attachment #8649285 - Flags: review?(masayuki)
Attachment #8648924 - Attachment is obsolete: true
Important regression, tracking
tracking-firefox40: ? → +
tracking-firefox41: ? → +
tracking-firefox42: ? → +
tracking-firefox43: ? → +
Comment on attachment 8649285 [details] [diff] [review] GetSysFontInfo should return MS Shell Dlg 2 for eFont_Field and eFont_List on Windows Oh, I realized that I hit non-related bug. Therefore, probably the cleaning up code isn't problem. However, I like this better for safer uplift. If you'd like to use the previous patch, it's okay for m-c but please use this for uplift.
Attachment #8649285 - Flags: review?(masayuki) → review+
url: https://hg.mozilla.org/integration/mozilla-inbound/rev/de7aa6b08234005ff1149a98a2e1c52868f993c5 changeset: de7aa6b08234005ff1149a98a2e1c52868f993c5 user: Jonathan Kew <jkew@mozilla.com> date: Tue Aug 18 17:21:38 2015 +0100 description: Bug 1194055 - GetSysFontInfo should return MS Shell Dlg 2 for eFont_Field and eFont_List on Windows. r=masayuki
I pushed the simplest version of the patch, to avoid any possibility of added risk. Assuming we don't see any unexpected problems when this goes out in Nightly, we can then consider uplift to earlier trains.
::sigh:: Good ol' ClearType... I'll re-land with the fuzz annotation on the problematic test adjusted, once I can catch inbound in an open state.
Flags: needinfo?(jfkthame)
https://hg.mozilla.org/mozilla-central/rev/de7aa6b08234
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox43: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
(In reply to Wes Kocher (:KWierso) from comment #75) > https://hg.mozilla.org/mozilla-central/rev/de7aa6b08234 We merged this around even though it got backed out for test failures?
status-firefox43: fixed → affected
Flags: needinfo?(wkocher)
Target Milestone: mozilla43 → ---
Ugh, thought I merged at a point before this landed... Here's a merged backout https://hg.mozilla.org/mozilla-central/rev/f384789a29dc
Status: RESOLVED → REOPENED
Flags: needinfo?(wkocher)
Resolution: FIXED → ---
I can reproduce this bug without Windows-Ja and with the hardware acceleration is disabled with the search box in page https://dl.dropboxusercontent.com/u/95157096/85f61cf7/vw28x3xavh.html. Screenshot https://dl.dropboxusercontent.com/u/95157096/85f61cf7/pmoli8vaer.png
url: https://hg.mozilla.org/integration/mozilla-inbound/rev/c260ae946b62fe941638850e4410ca8d68ea3749 changeset: c260ae946b62fe941638850e4410ca8d68ea3749 user: Jonathan Kew <jkew@mozilla.com> date: Tue Aug 18 17:21:38 2015 +0100 description: Bug 1194055 - GetSysFontInfo should return MS Shell Dlg 2 for eFont_Field and eFont_List on Windows. r=masayuki
https://hg.mozilla.org/mozilla-central/rev/c260ae946b62
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
status-firefox43: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Jonathan, do you have plans to uplift it? (aurora, beta or release?) We are probably going to do a 40.0.3 next week. If this patch is safe, we could take it. If we take it in the dot release, it would be great if we could land this fix in aurora & beta before to improve testing. Thanks
Flags: needinfo?(jfkthame)
Comment on attachment 8649285 [details] [diff] [review] GetSysFontInfo should return MS Shell Dlg 2 for eFont_Field and eFont_List on Windows Approval Request Comment [Feature/regressing bug #]: 1123654 [User impact if declined]: Default size of <input> elements on Japanese Windows systems is excessively wide; disrupts layout of some sites. [Describe test coverage new/current, TreeHerder]: We don't have automated test coverage on non-en-US systems. Manually tested (see comments above from masayuki, m_kato). [Risks and why]: Bug 1123654 will have affected the font used for <input> elements for many locales, and the fix here will affect them again, but for most locales/users the effect is so minor as to go unnoticed. There's inevitably some risk with any change that affects default font choices, in that some users may dislike the change; and changes to default metrics may disrupt layout of poorly-authored sites that made fragile assumptions. Note that the patch here will affect all locales (not only Japanese), though in most cases the changes will be quite minor (most users probably don't notice whether their fields are using Tahoma or Arial or Segoe UI or....); it's only in the Japanese case that we saw a drastic change in metrics, resulting in this bug being filed. [String/UUID change made/needed]: n/a :masayuki, :m_kato: please feel free to add any comments regarding whether we should uplift this, particularly from the point of view of the Japanese community as that's who is primarily affected here.
Flags: needinfo?(masayuki)
Flags: needinfo?(m_kato)
Flags: needinfo?(jfkthame)
Attachment #8649285 - Flags: approval-mozilla-release?
Attachment #8649285 - Flags: approval-mozilla-beta?
Attachment #8649285 - Flags: approval-mozilla-aurora?
Thank you, Jonathan. The main victim is our users who use Firefox on Win Vista or Win 7 since their Japanese default _UI_ font, Meiryo, has about twice wider than other fonts. Therefore, <input size="n"> becomes twice longer than Fx39. Of course, this bug isn't reproduced if web designer specifies width or max-width (or font-family) to all <input> elements. So, this bug is reproduced only when they access legacy websites which are typically not maintained anymore in most cases. Therefore, we need to do better in our side. The patch changes the default font of <input type="text"> and <option> in <select> to _dialog's_ default font which does not depend on the locale of Windows. Although, the new font is also different from the default font in Fx39, but the difference of the width/height is much less than current Fx40's. So, we can _reduce_ the number of the broken websites. In other words, this bug must not break the fix of bug 1123654 because its main purpose was applying modern font for all form controls. On the other hand, the patch applies better system font *only* for <input type="text"> and <option>.
Flags: needinfo?(masayuki)
Flags: needinfo?(jdaggett)
At first, many people has negative feedback for this change by bug 1123654. (The reason is comment #83) This is also web compatibility issue, for vs IE, or vs Chrome. I think that We should use same font against IE/Chrome as possible.
Flags: needinfo?(m_kato)
Comment on attachment 8649285 [details] [diff] [review] GetSysFontInfo should return MS Shell Dlg 2 for eFont_Field and eFont_List on Windows Let's take it now to have some coverage before the dot release. However, as we didn't have any bug report of this before the 40 release, I am not sure if we will get much feedback.
Attachment #8649285 - Flags: approval-mozilla-release?
Attachment #8649285 - Flags: approval-mozilla-release+
Attachment #8649285 - Flags: approval-mozilla-beta?
Attachment #8649285 - Flags: approval-mozilla-beta+
Attachment #8649285 - Flags: approval-mozilla-aurora?
Attachment #8649285 - Flags: approval-mozilla-aurora+
Added to the release notes "Fix a regression with some Japanese fonts used in the <input> field (1194055)"
relnote-firefox: --- → 40+

I got the same issue on the latest version of Firefox(89.0.2 (64-bit))
Same element but size are different between English OS and Japanese OS.
Could you guys take a look at this?
<a href="https://ibb.co/ry7k5GW"><img src="https://i.ibb.co/tCmBYMd/Untitled.png" alt="Untitled" border="0"></a>

Thank you.

Flags: needinfo?(jfkthame)

Minh, rather than adding comments to an old, closed bug please file a new bug with details of your issue. This bug has been resolved since several years ago now.

Flags: needinfo?(jfkthame)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: