New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 652146 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

serif, sans-serif, and default font on Android for lang=ja changed to monospace

Project Member Reported by kojii@chromium.org, Oct 3 2016

Issue description

STEPS:
1. Open http://output.jsbin.com/xigoxi

RESULT:
a. In "lang=ja" block, "default", "serif", and "sans-serif" are in mono-spaced font.
b. If the device lang is Japanese, "lang=" block as well.

NOTES:
Exact conditions not identified yet, but following is what I know:
1. Does not occur on:
   - Nexus 5X running N
   - Nexus 6P running N
2. Observed on:
   - Samsung Galaxy S6 Edge (au SCV31), running M
   - SONY Xperia Z4 tablet (SGP771), running M
3. Not observed on Mac/Win.
4. Stable (53.0.2785.124) is ok.
   Beta (54.0.2840.34), Canary 55.0.2875.3) are NG.
5. Pages without font in CSS is now very hard to read
   e.g. https://www.jwz.org/doc/worse-is-better.html
6. Affects gmail and other apps (confirmed in Feedly) too.

 

Comment 1 by kojii@chromium.org, Oct 3 2016

Labels: Needs-Bisect
Thought a regression from  issue 640077 , but the fix isn't in beta yet, so the issue is likely to be older.

I'm still not sure if this is M-specific, or device-specific. Can anyone add info for other devices running M?

Comment 2 by kojii@chromium.org, Oct 3 2016

Got a little trouble to setup, but succeeded to reproduce in locally built content_shell on the SONY device. Reverted  issue 640077  locally but it didn't fix.

Comment 3 by kojii@chromium.org, Oct 3 2016

Cc: e...@chromium.org
I figured this out, this is another side effect of  issue 609043 .

The Xperia has fonts.xml:
    <family lang="ja">
        <font weight="400" style="normal">SomcUDGothic-Regular.ttf</font>
    </family>
    <family lang="ja">
        <font weight="400" style="normal">NotoSansJP-Regular.otf</font>
    </family>
so "SomcUDGothic-Regular" is preferred over "NotoSansJP-Regular".

Before we fix  issue 609043 , it only appears when lang=ja, but now it appears when device is Japanese. I re-confirmed, in stable, "lang=" was ok but "lang=ja" was NG.

Now what to do is complicated. From Blink point of view, we're doing the right thing.

On the other hand, Galaxy and Xperia are two most common devices sold in Japan. M54 suddenly switching default font to mono-spaced can cause a bit of trouble.

Anyone, thoughts?

Comment 4 by drott@chromium.org, Oct 3 2016

Status: Available (was: Untriaged)

Comment 5 by kojii@chromium.org, Oct 3 2016

Labels: -Needs-Bisect M54

Comment 6 by kojii@chromium.org, Oct 3 2016

Labels: -Pri-2 Pri-1
Owner: kojii@chromium.org
Status: Assigned (was: Available)
Talked with a few people in Tokyo.

@nona and I thought this is M54 Release-Blocker, since, although this is device-specific issue, the end-user experience is quite bad. Observed in Xperia and Galaxy (two most common devices in Japan) scares us enough to think so.

Talked with @tkent then. He thinks we should fix this for M54, but is not important enough to delay the release because it only affects non-CJK characters and common users would not view English pages as often as we developers do.

Now the cause was identified, the fix shouldn't be hard. I'll work on it anyway, I hope whether this is RB or not shouldn't matter.

WIP: https://codereview.chromium.org/2390623002

Comment 7 by drott@chromium.org, Oct 3 2016

I consider it quite important as well, thanks very much for for spotting this and taking this on. Could you explain the fix proposal a bit more, I don't understand the approach.

Is this a regression after some of the locale improvements?

Comment 8 by drott@chromium.org, Oct 3 2016

So the SomcUDGothic is a monospaced font? Does it work correctly on Nexus? Would it make sense to special case this based on the Xperia's font configuration? Where does fontDescription.locale() come from vs. fontDescription.script()?

Comment 9 Deleted

Comment #9 quoted the wrong issue number, re-writing.

Confirmed on old Kyocera device running KitKat (4.4) too, so it looks like it's common to add a CJK font with monospace ASCII for lang=ja in fonts.xml.

> Is this a regression after some of the locale improvements?

No, this is a regression from  issue 609043 .

> So the SomcUDGothic is a monospaced font?

Yes. Like MS Gothic, it's a CJK font with monospace ASCII. Such font is useful in some use cases such as terminal or documents in primarily CJK scripts, since they provide ASCII in exact half-width, and many old people were used to such behavior, but is not appropriate to render non-CJK scripts.

> Where does fontDescription.locale() come from vs. fontDescription.script()

fontDescription.script() is a shortcut to fontDescription.localeOrDefault().script(). This returns the script of content locale, or the script of the default locale if the content locale is missing. It's been so for a long time, but the default locale was switched from "en" to the system locale in  issue 609043 .

fontDescription.locale() returns nullptr if the content locale is missing.

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/907fee664f47db6be9319c941a7e219ef4052c2b

commit 907fee664f47db6be9319c941a7e219ef4052c2b
Author: kojii <kojii@chromium.org>
Date: Mon Oct 03 15:16:56 2016

Re-limit the CJK font hack for Android to only when the content language is available

Android FontCache has a hack for CJK, put in 2013 or earlier. This hack
kicked in only when the content language is one of CJK until M53, but
the fix for  crbug.com/609043  unintentionally enabled it when the
content language is missing and the system language is one of CJK.

As a quick fix to ease the possible merge to M54, this patch limits the
hack to the same criteria as M53 without reverting  crbug.com/609043 .

A long term solution is to entirely remove this hack, but it will need
changes to Skia and possibly to fonts.xml as well. This will be worked
out separately.

BUG= 652146 

Review-Url: https://codereview.chromium.org/2390623002
Cr-Commit-Position: refs/heads/master@{#422424}

[modify] https://crrev.com/907fee664f47db6be9319c941a7e219ef4052c2b/third_party/WebKit/Source/platform/fonts/android/FontCacheAndroid.cpp

Labels: -M54 M-54 ReleaseBlock-Stable
Adding appropriate labels to ensure this is tracked for fixing before the M54 release.
Labels: Merge-Request-54
Status: Fixed (was: Assigned)
Confirmed the fix on today's dev 55.0.2880.3

Comment 14 by dimu@chromium.org, Oct 6 2016

Labels: -Merge-Request-54 Merge-Approved-54 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M54 (branch: 2840)
Project Member

Comment 15 by bugdroid1@chromium.org, Oct 6 2016

Labels: -merge-approved-54 merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6c440f50d160866d1063f87527c9c9e8c521d8df

commit 6c440f50d160866d1063f87527c9c9e8c521d8df
Author: Koji Ishii <kojii@chromium.org>
Date: Thu Oct 06 18:42:51 2016

Merge 2840: Re-limit the CJK font hack for Android to only when the content language is available

Android FontCache has a hack for CJK, put in 2013 or earlier. This hack
kicked in only when the content language is one of CJK until M53, but
the fix for  crbug.com/609043  unintentionally enabled it when the
content language is missing and the system language is one of CJK.

As a quick fix to ease the possible merge to M54, this patch limits the
hack to the same criteria as M53 without reverting  crbug.com/609043 .

A long term solution is to entirely remove this hack, but it will need
changes to Skia and possibly to fonts.xml as well. This will be worked
out separately.

BUG= 652146 

Review-Url: https://codereview.chromium.org/2390623002
Cr-Commit-Position: refs/heads/master@{#422424}
(cherry picked from commit 907fee664f47db6be9319c941a7e219ef4052c2b)

Review URL: https://codereview.chromium.org/2396143002 .

Cr-Commit-Position: refs/branch-heads/2840@{#665}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/6c440f50d160866d1063f87527c9c9e8c521d8df/third_party/WebKit/Source/platform/fonts/android/FontCacheAndroid.cpp

> > So the SomcUDGothic is a monospaced font?

> Yes. Like MS Gothic, it's a CJK font with monospace ASCII.

Sony should know better than that. They should not put 'bi-width' font for ja in fonts.xml for sans-serif/serif.  

Windows has both MS P Gothic/Mincho (ASCII is proportional) and MS Gothic/Mincho (ASCII is exactly half as wide as CJK; bi-width) so that what Sony is different from what Windows has. 

Maybe, we have to skip 'bi-width' font and pick the first proportional font when scanning fonts.xml for sans-serif and serif.  

Of course, scanning fonts.xml is not a sactioned way, either. This brings up yet another case for our internal bug ( b/26116537 ). 

> should not put 'bi-width' font for ja in fonts.xml

Yeah, but when 3 vendors have done that, we might need to pay attentions to it.

> Windows has both MS P Gothic/Mincho (ASCII is proportional)

"P" is also problematic because it made both ASCII and Kana proportional, which isn't suitable for CJ documents. It took ~10 years for MS to figure it out and switch to "proportional ASCII + fixed Kana" fonts, so early Android having the same problem isn't a surprise.

Appreciate your support to work on the long term solution on this.
Status: Verified (was: Fixed)
Verified fix on Galaxy S6 Edge(T-Mobile)/LRX22G, latest M54 release.
Project Member

Comment 19 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6c440f50d160866d1063f87527c9c9e8c521d8df

commit 6c440f50d160866d1063f87527c9c9e8c521d8df
Author: Koji Ishii <kojii@chromium.org>
Date: Thu Oct 06 18:42:51 2016

Merge 2840: Re-limit the CJK font hack for Android to only when the content language is available

Android FontCache has a hack for CJK, put in 2013 or earlier. This hack
kicked in only when the content language is one of CJK until M53, but
the fix for  crbug.com/609043  unintentionally enabled it when the
content language is missing and the system language is one of CJK.

As a quick fix to ease the possible merge to M54, this patch limits the
hack to the same criteria as M53 without reverting  crbug.com/609043 .

A long term solution is to entirely remove this hack, but it will need
changes to Skia and possibly to fonts.xml as well. This will be worked
out separately.

BUG= 652146 

Review-Url: https://codereview.chromium.org/2390623002
Cr-Commit-Position: refs/heads/master@{#422424}
(cherry picked from commit 907fee664f47db6be9319c941a7e219ef4052c2b)

Review URL: https://codereview.chromium.org/2396143002 .

Cr-Commit-Position: refs/branch-heads/2840@{#665}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/6c440f50d160866d1063f87527c9c9e8c521d8df/third_party/WebKit/Source/platform/fonts/android/FontCacheAndroid.cpp

Sign in to add a comment