1194055 - Size of <input> elements has changed in Firefox 40

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Brian Birtles (:birtles, Feb 26~ parental leave)]

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

Comment 1

[Kohei Yoshino [:kohei]]

Is this only in Japanese and Windows environment?

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(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)

Comment 3

[John Daggett (:jtd)]

(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".

Comment 4

[Alice0775 White]

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

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

[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.

Comment 6

[Jonathan Kew [:jfkthame]]

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?)

Comment 7

[Alice0775 White]

Attachment: Bug 1194055 input.html

AFAIK, default font is attached

Comment 8

[Alice0775 White]

Attachment: Bug 1194055 input.html

Comment 9

[Alice0775 White]

Attachment: Screenshot comparison between Edge, Firefox39 and Nightly43.0a01 on Windows10

Comment 10

[Alice0775 White]

Attachment: Screenshot comparison between Ie11, Firefox39 and Nightly43.0a01 on Windows7

Comment 11

[Jonathan Kew [:jfkthame]]

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?

Comment 12

[Alex]

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.

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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.

Comment 15

[Makoto Kato [:m_kato]]

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

Comment 16

[Kazumasa Hasegawa (Kazz)]

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?

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> Why not simply add "max-width" to the default style?

Specifying such rule may break the web...

Comment 18

[Kazumasa Hasegawa (Kazz)]

(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 ?

Comment 19

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

We cannot say "overflow: visible; always causes unexpected result".

Comment 20

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(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)

Comment 21

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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??

Comment 22

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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

Comment 23

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=141d00ae65eb

Comment 24

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: 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.

Comment 25

[John Daggett (:jtd)]

(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.

Comment 26

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(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.

Comment 27

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Oops, forgot to ni?.

Comment 28

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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.

Comment 29

[John Daggett (:jtd)]

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.

Comment 30

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(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?

Comment 31

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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.

Comment 32

[Sylvestre Ledru [:Sylvestre]]

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)

Comment 33

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(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...

Comment 34

[Jonathan Kew [:jfkthame]]

(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?

Comment 35

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(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.

Comment 36

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

[Tracking Requested - why for this release]: comment 5, comment 31 and comment 33.

Comment 37

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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.

Comment 38

[Jonathan Kew [: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

Comment 40

[Jonathan Kew [:jfkthame]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f9c2e55f8545

Comment 41

[Jonathan Kew [:jfkthame]]

Attachment: Workaround for excessively large xAvgCharWidth in Meiryo -- use width of 'zero' instead

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.

Comment 42

[Jonathan Kew [:jfkthame]]

: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...)

Comment 43

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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 44

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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?

Comment 45

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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).

Comment 46

[Kohei Yoshino [:kohei]]

https://developer.mozilla.org/en-US/Firefox/Releases/40/Site_Compatibility#HTML

Comment 47

[Masatoshi Kimura [:emk]]

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 48

[Makoto Kato [:m_kato]]

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?

Comment 49

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: screenshot of <input> and <input size="40">

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 50

[Makoto Kato [:m_kato]]

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.

Comment 51

[Makoto Kato [:m_kato]]

- 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.

Comment 52

[Makoto Kato [:m_kato]]

ah, test is invalid
When using Edge and IE11 on Windows 10, it is still MS UI/P Gothic.

Comment 53

[Makoto Kato [:m_kato]]

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.

Comment 54

[John Daggett (:jtd)]

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.

Comment 55

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(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.

Comment 56

[Makoto Kato [: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.

Comment 57

[Makoto Kato [: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...

Comment 58

[Jonathan Kew [:jfkthame]]

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.

Comment 59

[Jonathan Kew [:jfkthame]]

Attachment: GetSysFontInfo should return MS Shell Dlg 2 for eFont_Field and eFont_List on Windows

Does this help to address the issue on Japanese systems?

Comment 60

[Makoto Kato [:m_kato]]

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.

Comment 61

[Makoto Kato [:m_kato]]

Tested Windows 10 JA and Windows 7 JA.  It works.

Comment 62

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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 63

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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;

Comment 64

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> 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.

Comment 65

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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.

Comment 66

[Jonathan Kew [:jfkthame]]

(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?)

Comment 67

[Jonathan Kew [:jfkthame]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c1e2da183054

Comment 68

[Jonathan Kew [:jfkthame]]

Attachment: GetSysFontInfo should return MS Shell Dlg 2 for eFont_Field and eFont_List on Windows

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).

Comment 69

[Sylvestre Ledru [:Sylvestre]]

Important regression, tracking

Comment 70

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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.

Comment 71

[Jonathan Kew [:jfkthame]]

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/de7aa6b08234005ff1149a98a2e1c52868f993c5
changeset:  de7aa6b08234005ff1149a98a2e1c52868f993c5
user:       Jonathan Kew <[e-mail]>
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

Comment 72

[Jonathan Kew [:jfkthame]]

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.

Comment 73

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/50989f71e64b for Win8 reftest failures:


https://treeherder.mozilla.org/logviewer.html#?job_id=12995865&repo=mozilla-inbound

Comment 74

[Jonathan Kew [:jfkthame]]

::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.

Comment 75

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/de7aa6b08234

Comment 76

[Ryan VanderMeulen [:RyanVM]]

(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?

Comment 77

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Ugh, thought I merged at a point before this landed...

Here's a merged backout https://hg.mozilla.org/mozilla-central/rev/f384789a29dc

Comment 78

[blinky]

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

Comment 79

[Jonathan Kew [:jfkthame]]

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/c260ae946b62fe941638850e4410ca8d68ea3749
changeset:  c260ae946b62fe941638850e4410ca8d68ea3749
user:       Jonathan Kew <[e-mail]>
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

Comment 80

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c260ae946b62

Comment 81

[Sylvestre Ledru [:Sylvestre]]

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

Comment 82

[Jonathan Kew [: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.

Comment 83

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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>.

Comment 84

[Makoto Kato [:m_kato]]

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.

Comment 85

[Sylvestre Ledru [:Sylvestre]]

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.

Comment 86

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/224809529b39

Comment 87

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/309e4b0d0214

Comment 88

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/6b931df9c6e1

Comment 89

[Sylvestre Ledru [:Sylvestre]]

Added to the release notes
"Fix a regression with some Japanese fonts used in the <input> field (1194055)"

Comment 90

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

We now have release candidate builds if anyone cares to verify this:
- 40.0.3 - http://ftp.mozilla.org/pub/firefox/nightly/40.0.3-candidates/build1/
- ESR 38.2.1 - http://ftp.mozilla.org/pub/firefox/nightly/38.2.1esr-candidates/build2/

Comment 91

[Minh]

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.

Comment 92

[Jonathan Kew [: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.