[webkit-reviews] review requested: [Bug 20797] add Settings entries for per-script/lang generic family : [Attachment 38565] patch addressing Eric's concerns

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 25 13:32:18 PDT 2009


Jungshik Shin <jshin at chromium.org> has asked  for review:
Bug 20797: add Settings entries for per-script/lang generic family
https://bugs.webkit.org/show_bug.cgi?id=20797

Attachment 38565: patch addressing Eric's concerns
https://bugs.webkit.org/attachment.cgi?id=38565&action=review

------- Additional Comments from Jungshik Shin <jshin at chromium.org>
Thanks a lot for the review !!! And, sorry for the late reply.	I addressed all
your concerns. 


Unlike the comment in UScriptCodes.h (which I copied from ICU), the Unicode
character database doesn't have ScriptNames.txt. Instead, it has Scripts.txt
listing scripts for all the characters, from which it's possible to generate
the list of script codes. However, the order will not be compatible with that
in ICU's common/unicode/uscript.h. The CLDR (Common Locale Data Repository)
also has a file from which we can derive the list. Again, the order will be
different. Apparently, ICU (in each successive version) adds new scripts to the
end to keep old values remain compatible.  I added comments to this effect to
ChangeLog and UScriptCodes.h. 

BTW, I'm a bit worried about exposing ScriptFontMap (typedef) in WebCore
namespace, which is not a big deal but I want to avoid. We can avoid that by
making two static helper functions class static. However, the setter helper
cannot be inline because it refers to a static helper in cpp file.  Either we
can leave them as in this patch or we can make them class static (but not
inline).


More information about the webkit-reviews mailing list