Marking this with the RB label as this is a regression issue in M55 & not seen in 8743.35.0/54.0.2840.33

Please feel free to change it if needed.

Very interesting. Noto Sans CJK {KR,SC,TC,JP} are still available and they're used in web pages. 

Somehow Chrome OS UI (non-contents/non-blink) picks up C-J serif fonts and Korean Nanum fonts. 

ohsima@: is there any change recently in CrOS UI regarding the font selection for the UI? 

In the past, IDS_UI_FONT_FAMILY_CROS is passed to InitDefaultFontList() (and the cs.chromium.org still shows that code). IDS_UI_FONT_FAMILY_CROS has a locale-dependent list of font families and a font size. 

https://cs.chromium.org/chromium/src/ui/strings/translations/app_locale_settings_ko.xtb

etc does have the correct value for IDS_UI_FONT_FAMILY_CROS and it's still referred to in  ResourceBundle::InitDefaultFontList()

void ResourceBundle::InitDefaultFontList() {
#if defined(OS_CHROMEOS)
  std::string font_family = base::UTF16ToUTF8(
      GetLocalizedString(IDS_UI_FONT_FAMILY_CROS));
  gfx::FontList::SetDefaultFontDescription(font_family);

  // TODO(yukishiino): Remove SetDefaultFontDescription() once the migration to
  // the font list is done.  We will no longer need SetDefaultFontDescription()
  // after every client gets started using a FontList instead of a Font.
  gfx::PlatformFontLinux::SetDefaultFontDescription(font_family);
#else
  // Use a single default font as the default font list.
  gfx::FontList::SetDefaultFontDescription(std::string());
#endif
}

sorry for a typo when referring to you in the previous comment, oshima@. 

+kojii@

No, I'm not aware of any change.

kojii@, do you know anything about this?

oshima@, no, not that I know of. The one I was working on hasn't landed yet.

 Issue 646280  has been merged into this issue.

Could be on chromeos side change, not chrome.

warx@, can you run 55 chrome on 54 chromeos, 54 chrome on 55 chromeos and see which broke this?

To help bisecting, I'm rewriting my comment 4 again with a bit more details. 

1. The native UI areas in CJK locales are supposed to use font-spec specified via
IDS_UI_FONT_FAMILY_CROS.  For zh-CN, zh-TW, ja and ko, the spec is "Roboto, Noto Sans CJK {SC,TC,JP,KR}, 13px".  

2. However, different fonts (Serif font for C and J and 'Nanum Gothic' font for K) are used in the native UI areas. 

3. Web contents (both web pages and web UI) are fine.

BTW, it's pretty likely that the native UI font selection is also broken for other languages written in scripts *other than* Latin-Greek-Cyrillic. That is, the native UI fonts for Thai, Hindi (and all other Indic), Arabic, etc are likely to be wrong as well. In other words, any language NOT covered by Roboto can be broken. 

I tried to run 55 chrome (tot) on 54 chromeos (8743.13.0) and still saw the issue. Is 8743.13.0 old enough?

For 54 chrome on 55 chromeos, I had some problem building 54 chrome. I will do it later today. 

I still have the issue building 54 chrome on chrome sdk.

Here is what I got. I checked code_generator_v8.py and saw some revisions there.

I don't know how to fix this. Anyone can help give a try? Thanks!

(sdk veyron_speedy R54-8741.0.0) warx@warx0 ~/chromium/src $ ninja -C out_veyron_speedy/Release -j1500 chrome chrome_sandbox nacl_helper
ninja: Entering directory `out_veyron_speedy/Release'
[159/20845] ACTION //third_party/WebKit/Source/bindings/scripts:cached_jinja_templates(//build/toolchain/cros:target)
FAILED: gen/blink/bindings/scripts/cached_jinja_templates.stamp 
python ../../third_party/WebKit/Source/bindings/scripts/code_generator_v8.py gen/blink/bindings/scripts gen/blink/bindings/scripts/cached_jinja_templates.stamp
Traceback (most recent call last):
  File "../../third_party/WebKit/Source/bindings/scripts/code_generator_v8.py", line 499, in <module>
    sys.exit(main(sys.argv))
  File "../../third_party/WebKit/Source/bindings/scripts/code_generator_v8.py", line 489, in main
    jinja_env.get_template(template_filename)
  File "/usr/local/google/home/warx/chromium/src/third_party/jinja2/environment.py", line 791, in get_template
    return self._load_template(name, self.make_globals(globals))
  File "/usr/local/google/home/warx/chromium/src/third_party/jinja2/environment.py", line 765, in _load_template
    template = self.loader.load(self, name, globals)
  File "/usr/local/google/home/warx/chromium/src/third_party/jinja2/loaders.py", line 135, in load
    globals, uptodate)
  File "/usr/local/google/home/warx/chromium/src/third_party/jinja2/environment.py", line 917, in from_code
    exec(code, namespace)
  File "/usr/local/google/home/warx/chromium/src/third_party/WebKit/Source/bindings/templates/utilities.cpp", line 2, in <module>
    {# This indirection is just to avoid spurious white-space lines. #}
ImportError: cannot import name make_logging_undefined
[1657/20845] CC obj/third_party/sqlite/chromium_sqlite3/sqlite3.o^C
ninja: build stopped: interrupted by user.

Jinja2 was updated at https://crrev.com/2316103002 .
Looks like you're having a mismatch of the version.  If third_party/jinja2/ is version 2.7.1, then upgrade it to 2.8.  If it's 2.8, then downgrade it to 2.7.1.

BTW, I'll be offline for a week from now.  Sorry.

re comment 10, 
(1) 55 chrome (tot) on 54 chromeos (8743.13.0) -> can see the bug
(2) 54 chrome (54.0.2840.43) on 55 chromeos (8844.0.0) -> cannot see the bug
(3) 55 chrome (tot) on 55 chromeos (8844.0.0) -> can see the bug

So this is a chrome side bug, not chromeos.

I try to run bisect and it behaviors a little bit different from the build on device. I consider the first attached one as good and the second one as bad.

The bisect results are: https://chromium.googlesource.com/chromium/src/+log/55e6683f5fad4e9ee67af77ffc2300cf4198c195..bf5f253616dfb7f6bb1f3e490b2a0804561dea5d

assign back to oshima@, as I don't know which one is the suspected CL.

Deleted: Screenshot from 2016-10-05 11:46:16.png 18.0 KB

	Screenshot from 2016-10-05 11:46:16.png 18.0 KB View Download

Deleted: Screenshot from 2016-10-05 11:47:04.png 17.8 KB

	Screenshot from 2016-10-05 11:47:04.png 17.8 KB View Download

drive-by, 

looking at the list and something that remotely touches fonts; 
https://codereview.chromium.org/2382443007 has a call GetLabelFontListAt(index), whatever that means.

hmmm

#16, 17 this started happening way before these changes. warx@ can you run binary bisect again.

I've started bisect so I'll take this one.

bisected to this https://codereview.chromium.org/2288093002

drott@ can you look into this?

The change in questions switches on filtering out Type 1 fonts. The filtering itself was added in https://codereview.chromium.org/2280053002 - Check line 610 of that CL. It adds FcPatterns to check for the font format, and as such the TrueType pattern is added first, then CFF. Noto Sans CJK probably has CFF outlines and is thus put lower in the result list. I am surprised that this shakes up the UI font selection.

Jungshik, any pointers where the UI font matching happens? How can I reproduce this? It'd be good to have someone from the CrOS side help working on this, as I probably wont't have the time soon to set up a CrOS environment.

It happens with _any_ CFF font. 
Just use "Customize Fonts" and set "Standard font" (and the other fonts) to any
CFF font. 
It will be ignored (even in the GUI) and a TrueType fallback will be used.

Indeed the CL from above does not work as intended, working on a fix. 

Fix proposal here https://codereview.chromium.org/2410063002

The change landed in Skia ToT, compare https://skia.googlesource.com/skia.git/+/1c7805bb1c3030efd7a4c848133b4f19285a9df1 . Could somebody help explaining to which branch I have to merge the change and how? Thanks.

drive-by: 
I suspect there's already skia M55 branch https://skia.googlesource.com/skia.git/+log/chrome/m55
so you should try cherry picking there.

These processes look reasonable to me: 
sites/skia/development-on-a-chrome-branch
sites/skia/chrome-merge-process

Your change meets the bar and is auto-approved for M55 (branch: 2883)

warx@ or others, could you check with Chrome ToT after r424490 on CrOS to see if this is fixed before I merge? Thank you.

tot device build seen bug fixed

warx@, thank you very much for verifying. Okay, I will merge to skia's chrome/m55 branch. 

dimu@, is it enough to merge to this third_party/skia branch or do I need to do anything on Chrome's 2883 M55 branch to get this into CrOS beta?

Thanks, Dominik, for fixing this issue. 

re comment 21: 

> The change in questions switches on filtering out Type 1 fonts. The filtering itself was added in https://codereview.chromium.org/2280053002 - Check line 610 of that CL. It adds FcPatterns to check for the font format, and as such the TrueType pattern is added first, then CFF. Noto Sans CJK probably has CFF outlines and is thus put lower in the result list. I am surprised that this shakes up the UI font selection.

Yes, Noto Sans CJK is OTF with CFF table (CFF outlines). Anyway, both formats are allowed so that I'm, too, surprised. 

It looks like what's specified via IDS_UI_FONT_FAMILY_CROS (comment 11) is ignored and the first font in the filtered list covering characters is used. 

> Jungshik, any pointers where the UI font matching happens? How can I reproduce this?

In the past, the UI font selection was delegated to Pango in a sense. What's exactly done these days, I'm not sure. We have to track it down and make it honor IDS_UI_FONT_FAMILY_CROS. 

oshima@: do you know where the font for native UI is selected? 

> dimu@, is it enough to merge to this third_party/skia branch or do I need to do anything on Chrome's 2883 M55 branch to get this into CrOS beta?

I guess you have to update buildspec (DEPS file) for branch 2883.

$ git clone https://chrome-internal.googlesource.com/chrome/tools/buildspec

and, roll skia to a revision you need in branches/2883/DEPS

BTW, reversing the fontcofnig filter rule order (CFF first and Truetype second) may have been a work-around as well. 
Nonetheless, we have to fix the root cause (perhaps in a separate bug) mentioned in comment 32. 

Okay, thanks, Jungshik, then I think I am good:
https://chrome-internal.googlesource.com/chrome/tools/buildspec/+/master/branches/2883/DEPS for Skia points to 
  'src/third_party/skia':
    (Var("git_url")) + '/skia.git@refs/heads/chrome/m55',

So, just merging to that Skia branch should be enough. 

https://codereview.chromium.org/2423433002 is the CL for merging to Skia chrome/m55

The above CL has landed, marking as fixed. 

re comment 32: 

https://codereview.chromium.org/2278143002 might have sth do with the UI font selection (hmm, that's in contents/). 

hshi@, do you know how native UI fonts are selected for different UI languages these days (in ozone? ) ? We cannot leave it up to the "mercy" of fontconfig.

I'll file a new bug (after doing some tests). 

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot