283461 - serif/sans-serif font pref not honoured correctly

Status: RESOLVED WORKSFORME

(Core :: Layout: Text and Fonts, defect)

Description

[Simon Montagu :smontagu]

Since the fix for bug 95227, all languages follow the default for serif or
sans-serif set for the language of the user's locale, not the pref. for
individual languages.

Comment 1

[Simon Montagu :smontagu]

Attachment: Patch for Windows

I don't yet know if this problem exists on other platforms.

Comment 2

[Jungshik Shin]

Xft might have the same problem. Need to test...

Comment 3

[Jungshik Shin]

Attachment: test case

'default proportional' for each langGroup should be rendered with the default
proportional font for that langGroup while lines with 'serif' and 'sans-serif'
should be rendered with fonts designated as 'serif' and 'sans-serif' for that
group. 

If 'allow documents to use other fonts' ('always use my fonts') is checked (not
checked), all three lines for each language should be rendered with the default
proportional font for the langGroup.

Comment 4

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

The test-case is wrong.
The font-switching is not related the lang attribute.

If your prefs is default setting, when you select the page's encoding is Western,
the default font is "serif". when you select it is Japanese or Korean or Chinese,
the default font is "sans-serif".

Comment 5

[Jungshik Shin]

Attachment: test case 2 (with more langGroups)

Masayuki, I'm sorry I don't understand what you meant. I'm afraid you're
mistaken about what lang and font selection is supposed to interact in gecko.

Comment 6

[Jungshik Shin]

(In reply to comment #5)
oops
> mistaken about what lang and font selection is supposed to interact in gecko.
  about the way lang and font selection are supposed to interact ....

Comment 7

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

I think this bug is INVA.
Currently, Gecko is not change default font in a document.
The pref defines the font-family of root element. not per element.

if the document is decoding ISO-8859-1(Western), the font-family of document's
root element is set by "font.default.x-western". And the elements in the
document are inherited the font-family.

Comment 8

[Jungshik Shin]

(In reply to comment #4)
Ok.now I got what you're trying to say.
> The test-case is wrong.
> The font-switching is not related the lang attribute.

 Sorry they're closely related in gecko and that's what W3C wants us to do. 

> If your prefs is default setting, when you select the page's encoding is Western,
> the default font is "serif". .....

 We falls back to to the character encoding of a document only when the language
is not explicitly specified. When it is explicitly specified, we always use that. 

Your comment #7 clearly shows that this bug is valid. What you wrote is NOT what
we're supposed to do. 

Comment 9

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

if we want to switch the font-family per element, I think that we should create
a new generic font name that is "-moz-default".

if |mLangGroup| is "-moz-default", we should refer the lang attribute of current
element.

Comment 10

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

My idea is here. 
bug 95227 comment 88

And see following comment of rbs.
bug 95227 comment 87

Comment 11

[Simon Montagu :smontagu]

For a real-world example, look at http://unicode.org/iuc/iuc10/x-utf8.html

It seems clear to me that each block in that page should be rendered in the
default font for the language.

Comment 12

[Jungshik Shin]

See bug 95227 comment #91. This bug is exactly about what rbs wrote we need to
do. Simon's patch for Windows is fixing it down in the Gfx land. It may or may
not be a good idea.

re : comment #11
Oh, yeah. How could I have forgotten about that famous example? 

This is not a typical regression, but we used use the 10th Unicode conference
page to boast of our ability to select fonts based on lang. Now  it doesn't work
any more. Masayuki, you certainly don't like  'ransom-note style' rendering of
the Simplified Chinese section on Japanese Windows, do you?
 

Comment 13

[Jungshik Shin]

oops. sorry. lang still works, but the default proportional font selection
doesn't   follow the setting for 'lang' but follow the setting for the 'root'
element (in case of UTF-* page without 'lang' specified, that would be the
default font for the 'locale lang')

Comment 14

[rbs]

> Simon's patch for Windows is fixing it down in the Gfx land. It may or may
> not be a good idea.

I don't think it is a good idea to diverge GfxWin there. As I suggested in the
other bug, just add a langGroup parameter to nsPresContext::GetDefaultFont(),
then catch all the call sites with the help of the compiler, and adjust them
accordingly. (This is reminiscent of how |SetFontFromStyle| came to being in
nsFrame.cpp.)

Masayuki: Adding a |-moz-default| langGroup doesn't make much sense to me. I
don't even see how it is going to do what you want (bug 95227 comment 88).

You can achieve what you said with "inherit":
<p lang="ja">
  <span lang="en" style="font-family: inherit">english</span>
</p>

CSS authors that want this behavior explicitly will/should do that.

You suggested that Gecko should make <span lang="en"> inherit the Japanese font
by default. If this is the case, it becomes *very* difficult for an author to
automatically let embedded English elements use the user's default English
font... (The same goes for other languages for that matter.)

The natural (intuitive) reaction that users will have in front of a
multi-language document is going to be what Simon experienced in comment 11.

The spec even has an example to that effect:
http://www.w3.org/TR/REC-CSS2/fonts.html#font-family-prop

"The richer selector syntax of CSS2 may be used to create language-sensitive
typography. For example, some Chinese and Japanese characters are unified to
have the same Unicode codepoint, although the abstract glyphs are not the same
in the two languages.

*:lang(ja-jp) { font: 900 14pt/16pt "Heisei Mincho W9", serif }
*:lang(zh-tw) { font: 800 14pt/16.5pt "Li Sung", serif }

This selects any element that has the given language - Japanese or Traditional
Chinese - and requests the appropriate font."

So, when the font-family is missing, it is clear that the nicest default font of
the language is the best.

By providing good default fonts that are freely customizable via prefs, and the
added ability of inheriting if authors say so, it sounds to me that this is the
most flexible approach.

Comment 15

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

I don't think the patch is good idea.
If we change so, the computed value of font-family and actural font are not much.

For example:

<html>
  <body>
    <p lang="en">this paragraph is English text</p>
  </body>
</html>

if the document is decoded by Shift_JIS(Japanese), the computed value of
font-family of root element is "sans-serif".

font-family of the p element is inherited its ancestor. In this case, the
ancestor is root element. i.e., computed value of font-family of the p element
is "sans-serif".

But if pref font switch per element, actual result is serif.
It is not good. Because the computed style is "sans-serif".

I think that want of this bug is very natural.
But it is not conformance for CSS style system.

Comment 16

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

At the beginning, I agreeing of this bug.
But I changed my meditation after I find the conflict of the computed value and
actural font.

Comment 17

[rbs]

> But if pref font switch per element, actual result is serif.
> It is not good. Because the computed style is "sans-serif".

That's a weakness of Simon's patch.

If nsPresContext::GetDefaultFont() is changed as I indicated, the computed style
will match with the rendered style. This comes from the fact that it is
nsPresContext::GetDefaultFont() that really feeds the Style System initially.
Then, the Style System passes it to Gfx, giving a clean cross-platform solution
rather than a hack specific to GfxWin.

Comment 18

[Simon Montagu :smontagu]

Talking of nsPresContext::GetDefaultFont(), can someone explain to me why we have

#ifdef _IMPL_NS_LAYOUT
  const nsFont* GetDefaultFont(PRUint8 aFontID) const
  { return GetDefaultFontInternal(aFontID); }
#else
  const nsFont* GetDefaultFont(PRUint8 aFontID) const
  { return GetDefaultFontExternal(aFontID); }
#endif
http://lxr.mozilla.org/seamonkey/source/layout/base/nsPresContext.h#242

but GetDefaultFontExternal() just returns GetDefaultFontInternal() ?
http://lxr.mozilla.org/seamonkey/source/layout/base/nsPresContext.cpp#952

Comment 19

[rbs]

Looks like a deCOMtamination trickery, per CVS blame.

BTW, I am afraid you might have to undo bug 118918. The font needs to be copied
to a fresh struct which is initialized from prefs each time. Fortunately, the
perf hit from this was found to be marginal.

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #18)
The "external" version is virtual so calling it does not add a linkage
dependency. The "internal" version is nonvirtual so if you call it, you have to
be able to link directly to object files in gklayout, which means in practice
that you are gklayout.

Comment 21

[Matt]

I'm seeing this with sans-serif on the x-central-euro and x-cryillic langGroups.
 I've also seen this randomly happen on other pages that contain unicode
characters on a page without a charset/lang defined, so that might also be an
issue?  Maybe it'd be good to find a better way to handle undefined data via a
user pref (since the said site in question is in english using iso-8859-1, just
no headers proclaiming that).

Data: "-ee"
Renderred data: "-%u0435%u0435"

Comment 22

[Asa Dotzler [:asa]]

transferring nomination to 1.8b4. If we don't get it there, we don't get it for 1.1

Comment 23

[Martin Arndt]

-- Confirmed on FF 1.0.6 --

But it only happens once in the print-preview! - If I close the preview and
re-open it again, the sans-serif font is displayed (and also printed) correctly!

Changing it to confirmed since there are a couple of people who have seen this
bug?!?

Comment 24

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think that in order to fix this, we need to stop resolving the initial value of font-family to 'serif' or 'sans-serif' during value computation, which means not calling aPresContext->GetDefaultFont(kPresContext_DefaultVariableFont_ID, ...) from nsRuleNode.cpp or nsStyleStruct.cpp.  Instead, we should modify
https://hg.mozilla.org/mozilla-central/file/95c76c3b0172/gfx/src/nsFont.h#l28
to distinguish kGenericFont_moz_variable from kGenericFont_NONE, and then start using kGenericFont_moz_variable as a computed value in the font-family list to represent a font that needs to be resolved to the language's default (either serif or sans-serif) later.  Making that distinction might in turn require various other changes.

This would then have to be resolved at some point after style computation.  It could presumably be done in AppendGenericFontFromPref in gfx/thebes/gfxPlatform.cpp, which I believe is where we add the actual families in the font.name.* and font.name-list.* prefs.  (But it's important to double-check that it's not *additionally* done someplace earlier.  It certainly used to, but this would depend on that not happening.)  It would also probably require doing the same thing in DoGetFontFamily in layout/style/nsComputedDOMStyle.cpp so that we report a valid CSS computed value (although that would expose that the inheritance doesn't quite work like CSS, so I'm not sure what's actually better).

Comment 25

[Lu Wei]

I can't use serif fonts on Thunderbird 31.7. In Options--display--formatting, the "default font" selection do not work.  What TB authors is only Sans-serif font setting in the "advanced..." menu, whether or not "Serif" is selected in the "proportional" down-selection. Is this the same bug?

Comment 26

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:jfkthame, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 27

[Jonathan Kew [:jfkthame]]

Jungshik's testcase here seems to render as expected these days; closing as WFM.

(If there are aspects of this that are still relevant, please file fresh reports accordingly.)