Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2390623002:
Re-limit the CJK font hack for Android to only when the content language is available (Closed)
|
Created:
4 years, 2 months ago by kojii Modified:
4 years, 2 months ago CC:
ajuma+watch_chromium.org, blink-reviews, blink-reviews-platform-graphics_chromium.org, Rik, chromium-reviews, danakj+watch_chromium.org, dshwang, drott+blinkwatch_chromium.org, krit, f(malita), jbroman, Justin Novosad, pdr+graphicswatchlist_chromium.org, rwlbuis, Stephen Chennney Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRe-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
Committed: https://crrev.com/907fee664f47db6be9319c941a7e219ef4052c2b
Cr-Commit-Position: refs/heads/master@{#422424}
Patch Set 1 #Patch Set 2 : Comments updated #Messages
Total messages: 25 (17 generated)
The CQ bit was checked by kojii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by kojii@chromium.org
The CQ bit was checked by kojii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by kojii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
Description was changed from ========== android-font-locale BUG=652146 ========== to ========== Re-limit the CJK font hack for Android to only when the content language is available Android FontCache has a hack for CJK, put 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 accidentally enabled it when the content language is missing and the system language is one of CJK. As a quick fix to ease the merge, this patch limits the hack to the same criteria as M53. 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 ==========
Description was changed from ========== Re-limit the CJK font hack for Android to only when the content language is available Android FontCache has a hack for CJK, put 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 accidentally enabled it when the content language is missing and the system language is one of CJK. As a quick fix to ease the merge, this patch limits the hack to the same criteria as M53. 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 ========== to ========== Re-limit the CJK font hack for Android to only when the content language is available Android FontCache has a hack for CJK, put 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, this patch limits the hack to the same criteria as M53. 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 ==========
Description was changed from ========== Re-limit the CJK font hack for Android to only when the content language is available Android FontCache has a hack for CJK, put 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, this patch limits the hack to the same criteria as M53. 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 ========== to ========== 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, this patch limits the hack to the same criteria as M53. 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 ==========
Description was changed from ========== 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, this patch limits the hack to the same criteria as M53. 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 ========== to ========== 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. 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 ==========
kojii@chromium.org changed reviewers: + drott@chromium.org, eae@chromium.org
PTAL. I'm going to write a separate doc to explain the problem better. Since the removal of the hack would require Skia and possibly Android help, we need wider audiences to understand the problem. But I lost my access to Samsung device until next week, and the discussion shouldn't be short. I'd like to get an approval to merge this to M54 (sent a separate e-mail to PM,) appreciate your understanding to this quick fix.
LGTM
Description was changed from ========== 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. 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 ========== to ========== 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 ==========
Thank you!
The CQ bit was checked by kojii@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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 Committed: https://crrev.com/907fee664f47db6be9319c941a7e219ef4052c2b Cr-Commit-Position: refs/heads/master@{#422424} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/907fee664f47db6be9319c941a7e219ef4052c2b Cr-Commit-Position: refs/heads/master@{#422424}
Message was sent while issue was closed.
Thanks, Koji, the commit message made it clear what's going on, that was very helpful.
Message was sent while issue was closed.
LGTM |
