Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(47)

Issue 476123002: Update font preference to use Noto Sans CJK on CrOS (Closed)

Created:
6 years, 4 months ago by jungshik at Google
Modified:
6 years, 4 months ago
Reviewers:
Daniel Erat, sky
CC:
chromium-reviews, rginda+watch_chromium.org, mtomasz+watch_chromium.org, yoshiki+watch_chromium.org, arv+watch_chromium.org, jshin+watch_chromium.org
Project:
chromium
Visibility:
Public.

Description

Update font preference to use Noto Sans CJK on CrOS Noto Sans CJK is a new set of CJK fonts released by Google and Adobe on July 11. They have been developed with the harmony with Noto Sans (LGC) to be used together. For CJK, we don't need to use licensed fonts, 3rd party fonts, or Droid Sans Fallback any more. While I'm at it, replace 'Open Sans' (that CrOS deviced do not have) with 'Noto Sans UI' (the default UI font on CrOS for Latin-Greek-Cyrillic) and remove a reference to 'Droid Sans Fallback' (that is going to be removed in a separate CrOS CL). This CL will go in after the CrOS CL to add Noto Sans CJK is landed. ( https://chromium-review.googlesource.com/212624 ) BUG=399080 TEST=Start CrOS in 4 CJK locales and see the fonts set (noto Sans CJK <lang>) here are used. R=derat@chromium.org, sky@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=290138

Patch Set 1 #

Total comments: 2

Patch Set 2 : add missing changes for Japanese font pref #

Unified diffs Side-by-side diffs Delta from patch set Stats (+48 lines, -63 lines) Patch
M chrome/app/resources/locale_settings_chromiumos.grd View 3 chunks +12 lines, -12 lines 0 comments Download
M chrome/app/resources/locale_settings_google_chromeos.grd View 1 5 chunks +10 lines, -10 lines 0 comments Download
M chrome/app/resources/platform_locale_settings/locale_settings_cros_ja.xtb View 1 chunk +2 lines, -4 lines 0 comments Download
M chrome/app/resources/platform_locale_settings/locale_settings_cros_ko.xtb View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/app/resources/platform_locale_settings/locale_settings_cros_zh-CN.xtb View 1 chunk +4 lines, -6 lines 0 comments Download
M chrome/app/resources/platform_locale_settings/locale_settings_cros_zh-TW.xtb View 1 chunk +4 lines, -6 lines 0 comments Download
M chrome/browser/resources/gesture_config.css View 1 chunk +1 line, -1 line 0 comments Download
M ui/file_manager/file_manager/audio_player/css/audio_player.css View 1 chunk +1 line, -1 line 0 comments Download
M ui/file_manager/file_manager/audio_player/elements/track_list.css View 1 chunk +1 line, -1 line 0 comments Download
M ui/file_manager/gallery/css/gallery.css View 1 chunk +1 line, -1 line 0 comments Download
M ui/file_manager/video_player/css/video_player.css View 1 chunk +1 line, -1 line 0 comments Download
M ui/strings/translations/app_locale_settings_ja.xtb View 2 chunks +2 lines, -5 lines 0 comments Download
M ui/strings/translations/app_locale_settings_ko.xtb View 2 chunks +2 lines, -2 lines 0 comments Download
M ui/strings/translations/app_locale_settings_zh-CN.xtb View 2 chunks +3 lines, -6 lines 0 comments Download
M ui/strings/translations/app_locale_settings_zh-TW.xtb View 2 chunks +2 lines, -5 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
jungshik at Google
Daniel, can you take a look? Thanks
6 years, 4 months ago (2014-08-15 17:53:56 UTC) #1
Daniel Erat
nice! lgtm with a question https://codereview.chromium.org/476123002/diff/1/chrome/app/resources/platform_locale_settings/locale_settings_cros_zh-TW.xtb File chrome/app/resources/platform_locale_settings/locale_settings_cros_zh-TW.xtb (right): https://codereview.chromium.org/476123002/diff/1/chrome/app/resources/platform_locale_settings/locale_settings_cros_zh-TW.xtb#newcode8 chrome/app/resources/platform_locale_settings/locale_settings_cros_zh-TW.xtb:8: <translation id="IDS_SERIF_FONT_FAMILY">MSung B5HK</translation> is ...
6 years, 4 months ago (2014-08-15 18:11:01 UTC) #2
jungshik at Google
Thanks for the review. My answer is below. https://codereview.chromium.org/476123002/diff/1/chrome/app/resources/platform_locale_settings/locale_settings_cros_zh-TW.xtb File chrome/app/resources/platform_locale_settings/locale_settings_cros_zh-TW.xtb (right): https://codereview.chromium.org/476123002/diff/1/chrome/app/resources/platform_locale_settings/locale_settings_cros_zh-TW.xtb#newcode8 chrome/app/resources/platform_locale_settings/locale_settings_cros_zh-TW.xtb:8: <translation ...
6 years, 4 months ago (2014-08-15 18:32:39 UTC) #3
jungshik at Google
sky@chromium.org: Please review changes in ui/* and chrome/browser/*. Thank you.
6 years, 4 months ago (2014-08-15 19:02:43 UTC) #4
sky
LGTM
6 years, 4 months ago (2014-08-15 19:25:10 UTC) #5
jungshik at Google
On 2014/08/15 19:25:10, sky wrote: > LGTM Thank you. I'll add this to CQ once ...
6 years, 4 months ago (2014-08-15 23:29:51 UTC) #6
jungshik at Google
The CQ bit was checked by jshin@chromium.org
6 years, 4 months ago (2014-08-16 05:36:53 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jshin@chromium.org/476123002/1
6 years, 4 months ago (2014-08-16 05:37:58 UTC) #8
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_dbg on tryserver.chromium.linux ...
6 years, 4 months ago (2014-08-16 07:06:06 UTC) #9
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 4 months ago (2014-08-16 08:11:01 UTC) #10
commit-bot: I haz the power
Try jobs failed on following builders: android_dbg_triggered_tests on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_triggered_tests/builds/6422)
6 years, 4 months ago (2014-08-16 08:11:01 UTC) #11
jungshik at Google
6 years, 4 months ago (2014-08-16 08:33:03 UTC) #12
Message was sent while issue was closed.
Committed patchset #2 manually as 290138 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698