Summary: | Improve font fallback for 'common' characters in Chromium Win port | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jungshik Shin <jshin> | ||||||||
Component: | Layout and Rendering | Assignee: | Jungshik Shin <jshin> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | brettw, dglazkov | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Windows XP | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 25816, 113169 | ||||||||||
Attachments: |
|
Description
Jungshik Shin
2009-04-28 17:45:53 PDT
Created attachment 29946 [details]
patch
Brett, can you help Dimitri review the patch? Thanks. Comment on attachment 29946 [details] patch Looks good except for style comments: > +UScriptCode getScriptBasedOnUnicodeBlock(int ucs4) Add static modifier. > +UScriptCode getScript(int ucs4) Ditto. > + if (script <= USCRIPT_INHERITED || U_FAILURE(err)) > + script = getScriptBasedOnUnicodeBlock(ucs4); 4 spaces. Thank you for the review. (In reply to comment #3) > (From update of attachment 29946 [details] [review]) > Looks good except for style comments: > > > +UScriptCode getScriptBasedOnUnicodeBlock(int ucs4) > > Add static modifier. > > > +UScriptCode getScript(int ucs4) > > Ditto. They're currently inside an anonymous namespace. I'm aware that WebKit does not use anonymous namespace elsewhere. Do you think we have to follow the convention in this file? > > + if (script <= USCRIPT_INHERITED || U_FAILURE(err)) > > + script = getScriptBasedOnUnicodeBlock(ucs4); > > 4 spaces. I'll upload a check-in-ready patch once I hear back about the namespace issue above. Ah, I didn't see anonymous namespace in the context of the patch. LGTM then. Created attachment 30031 [details]
patch : same as before except for one indentation change (2->4)
Thanks again for review. This is ready for check-in.
Created attachment 30115 [details]
same as the previous except the test location
I moved two tests I'm adding to fast/text/international from fast/text.
Transferring r+.
Your patch was missing pixel results, but I added them before committing. Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog A LayoutTests/fast/text/international/danda-space.html A LayoutTests/fast/text/international/thai-baht-space.html A LayoutTests/platform/mac/fast/text/international/danda-space-expected.checksum A LayoutTests/platform/mac/fast/text/international/danda-space-expected.png A LayoutTests/platform/mac/fast/text/international/danda-space-expected.txt A LayoutTests/platform/mac/fast/text/international/thai-baht-space-expected.checksum A LayoutTests/platform/mac/fast/text/international/thai-baht-space-expected.png A LayoutTests/platform/mac/fast/text/international/thai-baht-space-expected.txt M WebCore/ChangeLog M WebCore/platform/graphics/chromium/FontUtilsChromiumWin.cpp Committed r43759 |