<rdar://problem/7860281> Accents are displayed displaced for decomposed text on the attached test case.
Created attachment 107733 [details] Test case
Created attachment 107741 [details] Allow Core Text to choose the fallback font for rendering a combining character sequence
Attachment 107741 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.cpp:53: _font is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.cpp:54: _character is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.cpp:55: _count is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.cpp:66: This { should be at the end of the previous line [whitespace/braces] [4] Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.cpp:80: This { should be at the end of the previous line [whitespace/braces] [4] Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.cpp:85: This { should be at the end of the previous line [whitespace/braces] [4] Total errors found: 6 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 107741 [details] Allow Core Text to choose the fallback font for rendering a combining character sequence Attachment 107741 [details] did not pass cr-mac-ews (chromium): Output: http://queues.webkit.org/results/9725215
(In reply to comment #4) > (From update of attachment 107741 [details]) > Attachment 107741 [details] did not pass cr-mac-ews (chromium): > Output: http://queues.webkit.org/results/9725215 This doesn’t seem to include the actual compile errors.
Comment on attachment 107741 [details] Allow Core Text to choose the fallback font for rendering a combining character sequence Attachment 107741 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9736126 New failing tests: fast/text/combining-character-sequence-fallback.html
Comment on attachment 107741 [details] Allow Core Text to choose the fallback font for rendering a combining character sequence Attachment 107741 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/9735296
Comment on attachment 107741 [details] Allow Core Text to choose the fallback font for rendering a combining character sequence It's not clear what pass/fail criteria for the test are. The expected result in this patch looks like a failure due to poor rendering of enclosed a, unless you know how bad it was before.
Comment on attachment 107741 [details] Allow Core Text to choose the fallback font for rendering a combining character sequence r=me I think you need fontDataForCombiningCharacterSequence to have #if PLATFORM(MAC) in Font.h. Fix the style errors also.
I know you don't use the method on other platforms, but I think it's cleaner to have #if PLATFORM(MAC) in the header as a clear indicator that it's a Mac-specific method.
(In reply to comment #8) > (From update of attachment 107741 [details]) > It's not clear what pass/fail criteria for the test are. The expected result in this patch looks like a failure due to poor rendering of enclosed a, unless you know how bad it was before. I have added some explanatory text to the test and, where possible, reference renderings.
(In reply to comment #9) > (From update of attachment 107741 [details]) > r=me > > I think you need fontDataForCombiningCharacterSequence to have #if PLATFORM(MAC) in Font.h. Not strictly needed, but done. > > Fix the style errors also. I think the style bot was confused by Objective-C++ in a .cpp file.
Fixed in <http://trac.webkit.org/r95391>.
(In reply to comment #4) > (From update of attachment 107741 [details]) > Attachment 107741 [details] did not pass cr-mac-ews (chromium): > Output: http://queues.webkit.org/results/9725215 I am hopeful that build.webkit.org will let me see the full build transcript now and I will be able to fix it ASAP.
Leopard build fixed in <http://trac.webkit.org/r95392>.
Snow Leopard build fix in <http://trac.webkit.org/r95393>.
Attempted Chromium Mac build fix in <http://trac.webkit.org/r95394>.
Additional Chromium Mac build fixes in <http://trac.webkit.org/r95395> and <http://trac.webkit.org/r95396>.
It appears that a test added by this patch is failing on Snow Leopard: http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r95747%20(33298)/platform/mac/fast/text/combining-character-sequence-fallback-pretty-diff.html Do we just need a rebaseline?
Hullo, We're getting some crash reports from users with stacks like this: Thread 0 *CRASHED* ( EXC_BAD_ACCESS / KERN_PROTECTION_FAILURE @ 0x00000000 ) 0x013415ed [Google Chrome Framework - ComplexTextControllerCoreText.mm:93] -[ChromiumWebCoreObjCWebCascadeList objectAtIndex:] 0x958b1191 [CoreFoundation + 0x00012191] CFArrayGetValueAtIndex 0x98a452a4 [CoreText + 0x000432a4] TFontCascade::CreateCascadeFallback(long) 0x98a4571f [CoreText + 0x0004371f] TFontCascade::CreateFallback(__CTFont const*, __CFString const*, CFRange) const 0x98a32513 [CoreText + 0x00030513] TGlyphEncoder::AppendUnmappedCharRun(CTRun*, CFRange, TGlyphList<TDeletedGlyphIndex>&, TGlyphList<TDeletedGlyphIndex>&, TFontCascade const&) 0x98a323cb [CoreText + 0x000303cb] TGlyphEncoder::RunUnicodeEncoderRecursively(CTRun*, adopted_t const&, CFRange, TGlyphList<TDeletedGlyphIndex>&, TGlyphList<TDeletedGlyphIndex>&, TFontCascade const&) 0x98a32677 [CoreText + 0x00030677] TGlyphEncoder::AppendUnmappedCharRun(CTRun*, CFRange, TGlyphList<TDeletedGlyphIndex>&, TGlyphList<TDeletedGlyphIndex>&, TFontCascade const&) 0x98a323cb [CoreText + 0x000303cb] TGlyphEncoder::RunUnicodeEncoderRecursively(CTRun*, adopted_t const&, CFRange, TGlyphList<TDeletedGlyphIndex>&, TGlyphList<TDeletedGlyphIndex>&, TFontCascade const&) 0x98a1da97 [CoreText + 0x0001ba97] TGlyphEncoder::RunUnicodeEncoder(CTRun*, adopted_t const&, CFRange, TGlyphList<TDeletedGlyphIndex>&, TFontCascade const&) 0x98a096e4 [CoreText + 0x000076e4] TGlyphEncoder::EncodeChars(CFRange, TAttributes const&, TGlyphList<TDeletedGlyphIndex>&) 0x98a3e577 [CoreText + 0x0003c577] TTypesetterUniChar::Initialize() 0x98a33c0e [CoreText + 0x00031c0e] CTLineCreateWithUniCharProvider 0x013420b0 [Google Chrome Framework - ComplexTextControllerCoreText.mm:232] WebCore::ComplexTextController::collectComplexTextRunsForCharactersCoreText 0x0133ec12 [Google Chrome Framework - ComplexTextController.cpp:345] WebCore::ComplexTextController::collectComplexTextRunsForCharacters 0x0133d67e [Google Chrome Framework - ComplexTextController.cpp:291] WebCore::ComplexTextController::collectComplexTextRuns 0x0133d41a [Google Chrome Framework - ComplexTextController.cpp:97] WebCore::ComplexTextController::ComplexTextController 0x0133d209 [Google Chrome Framework - ComplexTextController.cpp:82] WebCore::ComplexTextController::ComplexTextController 0x01344934 [Google Chrome Framework - FontComplexTextMac.cpp:112] WebCore::Font::floatWidthForComplexText 0x016e857d [Google Chrome Framework - RenderBlockLineLayout.cpp:1743] WebCore::textWidth 0x016e58b3 [Google Chrome Framework - RenderBlockLineLayout.cpp:2305] WebCore::RenderBlock::LineBreaker::nextLineBreak 0x016e2945 [Google Chrome Framework - RenderBlockLineLayout.cpp:1079] WebCore::RenderBlock::layoutRunsAndFloatsInRange 0x016e1001 [Google Chrome Framework - RenderBlockLineLayout.cpp:1048] WebCore::RenderBlock::layoutRunsAndFloats 0x016e6d72 [Google Chrome Framework - RenderBlockLineLayout.cpp:1334] WebCore::RenderBlock::layoutInlineChildren 0x016c9344 [Google Chrome Framework - RenderBlock.cpp:1265] WebCore::RenderBlock::layoutBlock 0x016c8cf7 [Google Chrome Framework - RenderBlock.cpp:1154] WebCore::RenderBlock::layout 0x016cf0f6 [Google Chrome Framework - RenderBlock.cpp:2020] WebCore::RenderBlock::layoutBlockChild 0x016cacfb [Google Chrome Framework - RenderBlock.cpp:1958] WebCore::RenderBlock::layoutBlockChildren 0x016c9361 [Google Chrome Framework - RenderBlock.cpp:1267] WebCore::RenderBlock::layoutBlock 0x016c8cf7 [Google Chrome Framework - RenderBlock.cpp:1154] WebCore::RenderBlock::layout 0x016cf0f6 [Google Chrome Framework - RenderBlock.cpp:2020] WebCore::RenderBlock::layoutBlockChild ...... (68 stack frames dropped.) This showed up quite recently and seems related to this patch Are you seeing anything similar? Do you think this is related? I do not have repro steps yet.
We're seeing this on 10.7 and 10.6 on a variety of URLs (no particular pattern to them).
(In reply to comment #20) > Hullo, > > We're getting some crash reports from users with stacks like this: > > Thread 0 *CRASHED* ( EXC_BAD_ACCESS / KERN_PROTECTION_FAILURE @ 0x00000000 ) > > 0x013415ed [Google Chrome Framework - ComplexTextControllerCoreText.mm:93] -[ChromiumWebCoreObjCWebCascadeList objectAtIndex:] Can you map that to an instruction and infer what is pointing to 0? > This showed up quite recently and seems related to this patch Are you seeing anything similar? No > Do you think this is related? A crash in code added in this patch is obviously related to the patch. Without knowing the conditions that trigger the crash it’s hard to tell whether they would have led to a different crash prior to this patch.
(In reply to comment #22) > (In reply to comment #20) > > Hullo, > > > > We're getting some crash reports from users with stacks like this: > > > > Thread 0 *CRASHED* ( EXC_BAD_ACCESS / KERN_PROTECTION_FAILURE @ 0x00000000 ) > > > > 0x013415ed [Google Chrome Framework - ComplexTextControllerCoreText.mm:93] -[ChromiumWebCoreObjCWebCascadeList objectAtIndex:] > > Can you map that to an instruction and infer what is pointing to 0? Disassembly around the crashing instruction: 0x3e2e25c2 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+130>: je 0x3e2e25ca <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+138> 0x3e2e25c4 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+132>: movl $0x0,(%eax) 0x3e2e25ca <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+138>: add $0x4,%eax 0x3e2e25cd <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+141>: add $0xfffffffc,%ecx 0x3e2e25d0 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+144>: jne 0x3e2e25c0 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+128> 0x3e2e25d2 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+146>: mov %edi,(%esi) 0x3e2e25d4 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+148>: mov 0x8(%ebp),%edi 0x3e2e25d7 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+151>: mov 0x4(%edi),%ecx 0x3e2e25da <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+154>: mov 0x28(%ecx),%eax 0x3e2e25dd <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+157>: mov %ebx,0x8(%esp) 0x3e2e25e1 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+161>: mov %ecx,0x4(%esp) 0x3e2e25e5 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+165>: mov %eax,(%esp) 0x3e2e25e8 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+168>: call 0x3e2879c0 <_ZNK7WebCore16FontFallbackList10fontDataAtEPKNS_4FontEj> ***** CRASH HERE ***** 0x3e2e25ed <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+173>: mov (%eax),%ecx 0x3e2e25ef <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+175>: mov 0x8(%edi),%edx 0x3e2e25f2 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+178>: mov %edx,0x4(%esp) 0x3e2e25f6 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+182>: mov %eax,(%esp) 0x3e2e25f9 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+185>: call *0x8(%ecx) 0x3e2e25fc <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+188>: add $0x28,%eax 0x3e2e25ff <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+191>: mov %eax,(%esp) 0x3e2e2602 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+194>: call 0x3e2d1820 <_ZNK7WebCore16FontPlatformData6ctFontEv> 0x3e2e2607 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+199>: mov %eax,(%esp) 0x3e2e260a <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+202>: call 0x3fd149e2 <dyld_stub_CTFontCopyFontDescriptor> 0x3e2e260f <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+207>: mov %eax,%esi 0x3e2e2611 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+209>: mov 0x14(%edi),%edi 0x3e2e2614 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+212>: test %esi,%esi 0x3e2e2616 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+214>: je 0x3e2e2620 <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+224> info registers eax 0x0 0 ecx 0x43d6f0 4445936 edx 0x8 8 ebx 0x2 2 esp 0xbfff8160 0xbfff8160 ebp 0xbfff8178 0xbfff8178 esi 0x182a40 1583680 edi 0x182a30 1583664 eip 0x3e2e25ed 0x3e2e25ed <-[ChromiumWebCoreObjCWebCascadeList objectAtIndex:]+173> It looks like _font is not NULL but the return value of _font->fontDataAt(index) is NULL > > Do you think this is related? > > A crash in code added in this patch is obviously related to the patch. Without knowing the conditions that trigger the crash it’s hard to tell whether they would have led to a different crash prior to this patch. Well I guess that was a silly question :). Here's a repro URL: http://www.pouria-alami.com/2011/08/blog-post_24.html. This crashes on ToT but not on an older build, so I think this is a regression.
(In reply to comment #23) > Here's a repro URL: http://www.pouria-alami.com/2011/08/blog-post_24.html. This crashes on ToT but not on an older build, so I think this is a regression. Thank you. I can now see what the problem is.
Filed bug 68737.
(In reply to comment #19) > It appears that a test added by this patch is failing on Snow Leopard: > http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r95747%20(33298)/platform/mac/fast/text/combining-character-sequence-fallback-pretty-diff.html > > Do we just need a rebaseline? Added Snow Leopard-specific expected results in <http://trac.webkit.org/r95909>.