Change the default serif font for Chinese

Change the default serif font for Chinese

This patch changes the default serif font for:
  Simplified Chinese: STSong -> Songti SC
  Traditional Chinese: LiSong Pro -> Songti TC

These fonts are available since 10.8, so we don't need to worry about
when these fonts are not available.

BUG=672140
Review-Url: https://codereview.chromium.org/2651873005
Cr-Commit-Position: refs/heads/master@{#446354}
Committed: https://chromium.googlesource.com/chromium/src/+/d14bd98f35e289c35e91ed6dcb3c1926ea58e059

[kojii, 2017-01-26 06:52:24]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-26 06:52:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kojii, 2017-01-26 07:00:38]

Description was changed from

==========
zhmac

BUG=672140
==========

to

==========
Change the default serif font for Chinese

This patch changes the default serif font for:
  Simplified Chinese: STSong -> Songti SC
  Traditional Chinese: LiSong Pro -> Songti TC

These fonts are available since 10.8 and that fallback is not needed.

BUG=672140
==========

[commit-bot: I haz the power, 2017-01-26 07:39:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-26 07:39:44]

Dry run: This issue passed the CQ dry run.

[kojii, 2017-01-26 07:44:40]

kojii@chromium.org changed reviewers:
+ drott@chromium.org

[kojii, 2017-01-26 07:44:41]

PTAL.

[drott, 2017-01-26 11:26:45]

This is only for new installations, right? Does this affect layout tests or
where is the default for layout tests coming from?

Can you clarify what you mean by "... and that fallback is not needed." Do you
mean: And we should avoid going down the fallback path from these default font
values to Songti?

[kojii, 2017-01-26 12:44:30]

On 2017/01/26 at 11:26:45, drott wrote:
> This is only for new installations, right?

Hmm, from the feedback I changed these settings before, I believe users will be
affected if they didn't change from the default, but I didn't read the code.

> Does this affect layout tests or where is the default for layout tests coming
from?

For LayoutTests, I believe testrunner sets different defaults.

> Can you clarify what you mean by "... and that fallback is not needed." Do you
mean: And we should avoid going down the fallback path from these default font
values to Songti?

Like the line before the change:
  <translation id="IDS_STANDARD_FONT_FAMILY">,PingFang SC,STHeiti</translation>
if the font is only available in, for instance, 10.9, we need ",A,B" format so
that "B" will be used if "A" isn't available.

Songti is added on 10.8, which is the lowest supported OS X version, so we can
simply change. I changed the description since "fallback" might not be
appropriate term here.

[kojii, 2017-01-26 12:45:10]

Description was changed from

==========
Change the default serif font for Chinese

This patch changes the default serif font for:
  Simplified Chinese: STSong -> Songti SC
  Traditional Chinese: LiSong Pro -> Songti TC

These fonts are available since 10.8 and that fallback is not needed.

BUG=672140
==========

to

==========
Change the default serif font for Chinese

This patch changes the default serif font for:
  Simplified Chinese: STSong -> Songti SC
  Traditional Chinese: LiSong Pro -> Songti TC

These fonts are available since 10.8, so we don't need to worry
when these fonts are not available.

BUG=672140
==========

[kojii, 2017-01-26 12:47:01]

Description was changed from

==========
Change the default serif font for Chinese

This patch changes the default serif font for:
  Simplified Chinese: STSong -> Songti SC
  Traditional Chinese: LiSong Pro -> Songti TC

These fonts are available since 10.8, so we don't need to worry
when these fonts are not available.

BUG=672140
==========

to

==========
Change the default serif font for Chinese

This patch changes the default serif font for:
  Simplified Chinese: STSong -> Songti SC
  Traditional Chinese: LiSong Pro -> Songti TC

These fonts are available since 10.8, so we don't need to worry about
when these fonts are not available.

BUG=672140
==========

[drott, 2017-01-26 13:48:44]

On 2017/01/26 at 12:44:30, kojii wrote:
> On 2017/01/26 at 11:26:45, drott wrote:
> > This is only for new installations, right?
> 
> Hmm, from the feedback I changed these settings before, I believe users will
be affected if they didn't change from the default, but I didn't read the code.

Sounds reasonable.


> > Does this affect layout tests or where is the default for layout tests
coming from?
> 
> For LayoutTests, I believe testrunner sets different defaults.

If they're different from SongTi, could you file a bug and change those to
SongTi in a separate CL as well?

> 
> > Can you clarify what you mean by "... and that fallback is not needed." Do
you mean: And we should avoid going down the fallback path from these default
font values to Songti?
> 
> Like the line before the change:
>   <translation id="IDS_STANDARD_FONT_FAMILY">,PingFang
SC,STHeiti</translation>
> if the font is only available in, for instance, 10.9, we need ",A,B" format so
that "B" will be used if "A" isn't available.
> 
> Songti is added on 10.8, which is the lowest supported OS X version, so we can
simply change. I changed the description since "fallback" might not be
appropriate term here.

Thanks, got it.

LGTM

[kojii, 2017-01-26 16:43:28]

Thanks for the review.

On 2017/01/26 at 13:48:44, drott wrote:
> > > Does this affect layout tests or where is the default for layout tests
coming from?
> > 
> > For LayoutTests, I believe testrunner sets different defaults.
> 
> If they're different from SongTi, could you file a bug and change those to
SongTi in a separate CL as well?

Oh I see your point now, good point...I believe layouttests doesn't have any,
and thus relies on system font fallback for Chinese, I'll check more. Probably
this is related.
https://crbug.com/535893

[kojii, 2017-01-26 16:43:33]

The CQ bit was checked by kojii@chromium.org

[commit-bot: I haz the power, 2017-01-26 16:44:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-26 16:48:39]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1485449013650660, "parent_rev":
"5c162d76d9db992f4d071b52c63b2a93a21eba8e", "commit_rev":
"d14bd98f35e289c35e91ed6dcb3c1926ea58e059"}

[commit-bot: I haz the power, 2017-01-26 16:49:11]

Description was changed from

==========
Change the default serif font for Chinese

This patch changes the default serif font for:
  Simplified Chinese: STSong -> Songti SC
  Traditional Chinese: LiSong Pro -> Songti TC

These fonts are available since 10.8, so we don't need to worry about
when these fonts are not available.

BUG=672140
==========

to

==========
Change the default serif font for Chinese

This patch changes the default serif font for:
  Simplified Chinese: STSong -> Songti SC
  Traditional Chinese: LiSong Pro -> Songti TC

These fonts are available since 10.8, so we don't need to worry about
when these fonts are not available.

BUG=672140

Review-Url: https://codereview.chromium.org/2651873005
Cr-Commit-Position: refs/heads/master@{#446354}
Committed:
https://chromium.googlesource.com/chromium/src/+/d14bd98f35e289c35e91ed6dcb3c...
==========

[commit-bot: I haz the power, 2017-01-26 16:49:12]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/d14bd98f35e289c35e91ed6dcb3c...