Bug 38224 – [chromium] Enable rendering of Ethiopic, Lao, Tibetan and a few other scripts on Win XP

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Jungshik Shin]

Currently, the last resort fallback font selection in Chromium Win is 'hard-coded'. For some scripts (Ethiopic, Lao, Tibetan, etc), those fonts listed are only available on Vista.  We're requested to release Chrome/Chromium in Ethiopic UI on Windows XP because it's the platform dominant in Ethiopia. 

To solve the problem for upcoming Chrome 5 release, I'm going to add a simple short-term  "fix" of going through multiple fonts (still hard-coded but having a high chance of availability) and picking the first one present on users' machines. While doing so, I do the same for some other scripts. 


This is for http://crbug.com/3752

[Jungshik Shin]

Oops. it's http://crbug.com/36752

[Jungshik Shin]

Created attachment 54471 [details]
patch

[Jungshik Shin]

Created attachment 54472 [details]
patch (same as before with copyright year updated)

[Eric Seidel (no email)]

Looks sane to me. CCing txt and windows folks though.

[Jungshik Shin]

Brett, can you take a quick look? If his nod and Eric's previous comment are sufficient for r+, it'd be great to get r+ from one of wk reviewers.

[Kent Tamura]

Comment on attachment 54472 [details]
patch (same as before with copyright year updated)

WebCore/platform/graphics/chromium/FontUtilsChromiumWin.cpp:51
 +                               fontName);
Nit: you don't need to fold "fontName);".


WebCore/platform/graphics/chromium/FontUtilsChromiumWin.cpp:112
 +      struct FontMap2 {
FontMap2 is not a meaningful name.
The name should contain "families" or "family list".


WebCore/platform/graphics/chromium/FontUtilsChromiumWin.cpp:150
 +      DEFINE_STATIC_LOCAL(Vector<Vector<String>* >, familyLists, ());
FontMap2 has a comma-separated value, the code parses it, and stores a static storage.  It is inefficient.
Why doesn't FontMap2 have a pointer to a string list?

e.g.
const UChar* malayalamFamilies[] = {L"AnjaliOldLipi", L"Kartika", L"Rachana", 0}
const static FontMap2 fontMap2[] = {
    {USCRIPT_MALAYALAM, malayalamFamilies},

[Jungshik Shin]

Created attachment 58201 [details]
patch updated 

I considered what tkent suggested and decided against it, but I don't remember what it was and I can't come up with any reason not to. So, I'm doing it with this patch.

[WebKit Review Bot]

Attachment 58201 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/platform/graphics/chromium/FontUtilsChromiumWin.cpp:168:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kent Tamura]

Comment on attachment 58201 [details]
patch updated 


WebCore/ChangeLog:5
 +          Make it possible to specify a list of fonts for per-script
Please add the bug URL in the ChangeLog entry.


WebCore/platform/graphics/chromium/FontUtilsChromiumWin.cpp:168
 +          while (*familyPtr != 0) {
Remove "!=0".

[Jungshik Shin]

Created attachment 58225 [details]
updated patch for commit (with style issues addressed)

@tkent, thank you for the review. Your concern was addressed and I'm carrying over r+.

[WebKit Commit Bot]

Comment on attachment 58225 [details]
updated patch for commit (with style issues addressed)

Rejecting patch 58225 from review queue.

[e-mail] does not have reviewer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py.

- If you do not have reviewer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have reviewer rights please correct the error in WebKitTools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  Due to bug 30084 the commit-queue will require a restart after your change.  Please contact [e-mail] to request a commit-queue restart.  After restart the commit-queue will correctly respect your reviewer rights.

[Kent Tamura]

Comment on attachment 58225 [details]
updated patch for commit (with style issues addressed)

re-set r+.

[Eric Seidel (no email)]

Comment on attachment 58201 [details]
patch updated 

Cleared Kent Tamura's review+ from obsolete attachment 58201 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[Jungshik Shin]

Sorry for the mess and thank you for r+ again.  Next time in the same situation, I'll just commit myself.

[WebKit Commit Bot]

Comment on attachment 58225 [details]
updated patch for commit (with style issues addressed)

Clearing flags on attachment: 58225

Committed r60953: <http://trac.webkit.org/changeset/60953>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.