|Summary:||Fonts with no vertical information should still render CJK glyphs upright|
|Product:||WebKit||Reporter:||Dave Hyatt <hyatt>|
|Component:||Layout and Rendering||Assignee:||Dave Hyatt <hyatt>|
|Severity:||Critical||CC:||abarth, dglazkov, eric, Kida, mitz, takano, webkit-ews, webkit.review.bot|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.5|
|Bug Depends on:|
Description Dave Hyatt 2010-11-12 11:44:06 PST
Fonts with no vertical information should still render CJK glyphs upright. See: http://people.mozilla.org/~jdaggett/tests/verticalmargins.html The font is Times-Roman, and it has Japanese glyphs but no vertical metrics. The glyphs should still render upright. Basically our code that is trying to make this decision purely based off the entire font is too simplistic. We need to do better.
Comment 1 Dave Hyatt 2010-11-12 13:16:13 PST
Created attachment 73770 [details] Patch Looking for feedback. Not ready to land yet.
Comment 2 WebKit Review Bot 2010-11-12 13:35:19 PST
Attachment 73770 [details] did not build on chromium: Build output: http://queues.webkit.org/results/5730023
Comment 3 Early Warning System Bot 2010-11-12 13:43:41 PST
Attachment 73770 [details] did not build on qt: Build output: http://queues.webkit.org/results/5740035
Comment 4 Yasuo Kida 2010-11-12 13:53:32 PST
I am not sure that this is an issue with missing vertical metric. Times font does not have Japanese glyphs and you are seeing them through the font fallback. The font used in the fallback is Hiragino, which has vertical metric. I guess the issue is around the handling of fallback fonts.
Comment 6 Eric Seidel (no email) 2010-11-12 14:59:17 PST
Attachment 73770 [details] did not build on chromium: Build output: http://queues.webkit.org/results/5750034
Comment 7 mitz 2010-11-12 15:07:20 PST
Comment on attachment 73781 [details] Patch Looks ok but would be good to test with a font that actually has CJK but not vertical support.
Comment 8 Early Warning System Bot 2010-11-12 15:15:38 PST
Attachment 73781 [details] did not build on qt: Build output: http://queues.webkit.org/results/5774035
Comment 9 Takumi Takano 2010-11-12 15:58:48 PST
It seems Font::isCJKIdeograph doesn't check CJK Extension B, C, D and CJK Compatibility Ideographs Supplement. They are mapped U+20000 and up. We need to count surrogate pairs here...
Comment 10 WebKit Review Bot 2010-11-12 16:14:41 PST
Attachment 73781 [details] did not build on chromium: Build output: http://queues.webkit.org/results/5840003
Comment 11 Dave Hyatt 2010-11-12 20:40:50 PST
Created attachment 73807 [details] Patch to address Takano's concerns and that fixes @font-face bugs as well.
Comment 12 WebKit Review Bot 2010-11-12 20:54:06 PST
Attachment 73807 [details] did not build on chromium: Build output: http://queues.webkit.org/results/5985004
Comment 13 mitz 2010-11-12 20:58:32 PST
Comment on attachment 73807 [details] Patch to address Takano's concerns and that fixes @font-face bugs as well. r=me, but try not to break cr-linux and efl
Comment 14 Early Warning System Bot 2010-11-12 20:58:39 PST
Attachment 73807 [details] did not build on qt: Build output: http://queues.webkit.org/results/6007004
Comment 15 Dave Hyatt 2010-11-12 21:08:12 PST
Going to keep using the bug to post patches to get the other ports building. This is just a matter of getting orientation() into everyone's FontPlatformData object, even if it just returns Horizontal on non-Mac platforms for now.