LocaleToScriptMappingICU.cpp treats certain script codes as a single script for font selection purposes. For example "Hira", "Kana", "Jpan", and "Hrkt" are all treated as "Hrkt". This way we only have to set fonts for the "Hrkt" script in Settings, and have it be used for all of the Japanese scripts. LocaleToScriptMappingDefault.cpp should do the same thing.
Created attachment 147042 [details] Patch
Uploaded a patch.
Comment on attachment 147042 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=147042&action=review Which ports use LocaleToScriptMappingICU and which use LocaleToScriptMappingDefault? > Source/WebCore/platform/text/LocaleToScriptMappingDefault.cpp:49 > + // Treat certain families of script codes as a single script for assigning a per-script font in Settings. > + static const UScriptCode japaneseScript = USCRIPT_KATAKANA_OR_HIRAGANA; > + static const UScriptCode koreanScript = USCRIPT_HANGUL; Why do we use separate variables for these rather than just inlining USCRIPT_KATAKANA_OR_HIRAGANA and USCRIPT_HANGUL?
It looks like Chromium uses Default.cpp. Should it use ICU.cpp instead?
(In reply to comment #3) > (From update of attachment 147042 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=147042&action=review > > Which ports use LocaleToScriptMappingICU and which use LocaleToScriptMappingDefault? Right now everyone uses Default. See <http://trac.webkit.org/changeset/110541> > > > Source/WebCore/platform/text/LocaleToScriptMappingDefault.cpp:49 > > + // Treat certain families of script codes as a single script for assigning a per-script font in Settings. > > + static const UScriptCode japaneseScript = USCRIPT_KATAKANA_OR_HIRAGANA; > > + static const UScriptCode koreanScript = USCRIPT_HANGUL; > > Why do we use separate variables for these rather than just inlining USCRIPT_KATAKANA_OR_HIRAGANA and USCRIPT_HANGUL? It seemed to have better readability this way and I thought the compiler would inline it anyway. I can change to inlining them.
(In reply to comment #5) > (In reply to comment #3) > > (From update of attachment 147042 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=147042&action=review > > > > Which ports use LocaleToScriptMappingICU and which use LocaleToScriptMappingDefault? > > Right now everyone uses Default. See <http://trac.webkit.org/changeset/110541> I see, so this would change font selection behavior for all ports. Is it possible to make a layout test demonstrating this change? Is this more consistent with other browsers like Firefox, IE or Opera? > > > Source/WebCore/platform/text/LocaleToScriptMappingDefault.cpp:49 > > > + // Treat certain families of script codes as a single script for assigning a per-script font in Settings. > > > + static const UScriptCode japaneseScript = USCRIPT_KATAKANA_OR_HIRAGANA; > > > + static const UScriptCode koreanScript = USCRIPT_HANGUL; > > > > Why do we use separate variables for these rather than just inlining USCRIPT_KATAKANA_OR_HIRAGANA and USCRIPT_HANGUL? > > It seemed to have better readability this way and I thought the compiler would inline it anyway. I can change to inlining them. I think it would be more consistent if they were all USCRIPT_.
(In reply to comment #6) > I see, so this would change font selection behavior for all ports. Is it possible to make a layout test demonstrating this change? Is this more consistent with other browsers like Firefox, IE or Opera? Oh, sorry, there is a test. I'm still curious about the second question. Also, are there sites currently using lang=ja-{Hira,Hrkt,Jpan,Kana}?
I'll make the change to use USCRIPT_. (In reply to comment #7) > (In reply to comment #6) > > I see, so this would change font selection behavior for all ports. Is it possible to make a layout test demonstrating this change? Is this more consistent with other browsers like Firefox, IE or Opera? I think the other browsers don't really look at script and just use the language. I tested on Firefox, and it uses the font preference for Japanese to render all of "lang=ja-{Hira,Hrkt,Jpan,Kana}", as this patch would do. > > Oh, sorry, there is a test. I'm still curious about the second question. Also, are there sites currently using lang=ja-{Hira,Hrkt,Jpan,Kana}? It seems really rare, if it happens at all. I actually only noticed this because my demo to show per-script font settings uses the script directly to show how sample text looks under different per-script font settings. I think no one would want different settings for Hira/Hrkt/Jpan/Kana though, so they should all be rendered with the same setting.
Created attachment 147460 [details] Patch
Comment on attachment 147460 [details] Patch Looks ok
Comment on attachment 147460 [details] Patch Thanks for the reviews.
Comment on attachment 147460 [details] Patch Clearing flags on attachment: 147460 Committed r120303: <http://trac.webkit.org/changeset/120303>
All reviewed patches have been landed. Closing bug.