Bug 76041

Summary: fast/text/unicode-variation-selector.html doesn't pass on Lion
Product: WebKit Reporter: Kenichi Ishibashi <bashi@chromium.org>
Component: TextAssignee: Kenichi Ishibashi <bashi@chromium.org>
Status: RESOLVED FIXED    
Severity: Normal CC: ap@webkit.org, mitz@webkit.org, webkit.review.bot@gmail.com
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
crash log
none
Patched font
none
Patch (patched font added)
none
Patch
none
Patch none

Description From 2012-01-11 02:26:09 PST
<http://trac.webkit.org/changeset/104545> broke fast/text/unicode-variation-selector.html on OS X Lion.
------- Comment #1 From 2012-01-11 12:05:59 PST -------
How does the failure look like?
------- Comment #2 From 2012-01-11 12:09:04 PST -------
Unmodified CJK glyph followed by a square.
------- Comment #3 From 2012-01-11 18:44:13 PST -------
Created an attachment (id=122149) [details]
Patch
------- Comment #4 From 2012-01-11 18:48:34 PST -------
(In reply to comment #3)
> Created an attachment (id=122149) [details] [details]
> Patch

I think this patch will fix the regression, but DRT often crashed when I tested. It looks like a bug of CoreText that I reported at <https://bugs.webkit.org/show_bug.cgi?id=74662>. I checked the input was valid. Here is a gdb log:

(gdb) bt
#0  0x00007fff8c74b9dc in std::equal_range<TFormat14UVSTable::UnicodeValueRange const*, int> ()
#1  0x00007fff8c74af7f in TFormat14UVSTable::Map ()
#2  0x00007fff8c74bf59 in TFormat12UTF16cmapTable::MapT<true> ()
#3  0x00007fff8c6f5d67 in TcmapTable::Map ()
#4  0x00007fff8c6f5afb in TBaseFont::GetGlyphsForCharacters ()
#5  0x00007fff8c700b35 in TUnicodeEncoder::EncodePortion ()
#6  0x00007fff8c700aca in TUnicodeEncoder::Encode ()
#7  0x00007fff8c716139 in TGlyphEncoder::RunUnicodeEncoderRecursively ()
#8  0x00007fff8c7164c4 in TGlyphEncoder::RunUnicodeEncoder ()
#9  0x00007fff8c716b2b in TGlyphEncoder::EncodeChars ()
#10 0x00007fff8c722c89 in TTypesetterUniChar::Initialize ()
#11 0x00007fff8c7183e5 in CTLineCreateWithUniCharProvider ()
#12 0x0000000102730efe in WebCore::SimpleFontData::canRenderCombiningCharacterSequence (this=0x10782d600, characters=0x108e56a40, length=3) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm:451
#13 0x000000010198260c in WebCore::Font::fontDataForCombiningCharacterSequence (this=0x106d4f7c0, characters=0x108e56a40, length=3, variant=WebCore::NormalVariant) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/platform/graphics/mac/FontComplexTextMac.cpp:152
#14 0x00000001014c2ce4 in WebCore::ComplexTextController::collectComplexTextRuns (this=0x7fff5fbf8608) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:278
#15 0x00000001014c2a55 in WebCore::ComplexTextController::ComplexTextController (this=0x7fff5fbf8608, font=0x106d4f7c0, run=@0x7fff5fbf9cb8, mayUseNaturalWritingDirection=true, fallbackFonts=0x7fff5fbf9fd0, forTextEmphasis=false) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:97
#16 0x00000001014c2774 in WebCore::ComplexTextController::ComplexTextController (this=0x7fff5fbf8608, font=0x106d4f7c0, run=@0x7fff5fbf9cb8, mayUseNaturalWritingDirection=true, fallbackFonts=0x7fff5fbf9fd0, forTextEmphasis=false) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:82
#17 0x00000001019820cd in WebCore::Font::floatWidthForComplexText (this=0x106d4f7c0, run=@0x7fff5fbf9cb8, fallbackFonts=0x7fff5fbf9fd0, glyphOverflow=0x7fff5fbf9fb8) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/platform/graphics/mac/FontComplexTextMac.cpp:112
#18 0x000000010196f76a in WebCore::Font::width (this=0x106d4f7c0, run=@0x7fff5fbf9cb8, fallbackFonts=0x7fff5fbf9fd0, glyphOverflow=0x7fff5fbf9fb8) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/platform/graphics/Font.cpp:188
#19 0x00000001026124f1 in WebCore::RenderText::widthFromCache (this=0x108e4b438, f=@0x106d4f7c0, start=0, len=3, xPos=0, fallbackFonts=0x7fff5fbf9fd0, glyphOverflow=0x7fff5fbf9fb8) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderText.cpp:766
#20 0x000000010260e4cd in WebCore::RenderText::computePreferredLogicalWidths (this=0x108e4b438, leadWidth=0, fallbackFonts=@0x7fff5fbf9fd0, glyphOverflow=@0x7fff5fbf9fb8) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderText.cpp:999
#21 0x000000010260da8b in WebCore::RenderText::computePreferredLogicalWidths (this=0x108e4b438, leadWidth=0) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderText.cpp:883
#22 0x000000010260da10 in WebCore::RenderText::maxLogicalWidth (this=0x108e4b438) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderText.cpp:874
#23 0x00000001026107fd in WebCore::RenderText::width (this=0x108e4b438, from=0, len=3, f=@0x106d4f7c0, xPos=269, fallbackFonts=0x0, glyphOverflow=0x0) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderText.cpp:1460
#24 0x00000001024687ea in textWidth (text=0x108e4b438, from=0, len=3, font=@0x106d4f7c0, xPos=269, isFixedPitch=false, collapseWhiteSpace=true) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlockLineLayout.cpp:1921
#25 0x000000010246465a in WebCore::RenderBlock::LineBreaker::nextLineBreak (this=0x7fff5fbfaef8, resolver=@0x7fff5fbfb168, lineInfo=@0x7fff5fbfb4a8, lineBreakIteratorInfo=@0x7fff5fbfafe0, lastFloatFromPreviousLine=0x0, consecutiveHyphenatedLines=0) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlockLineLayout.cpp:2514
#26 0x0000000102460230 in WebCore::RenderBlock::layoutRunsAndFloatsInRange (this=0x108e44c58, layoutState=@0x7fff5fbfb480, resolver=@0x7fff5fbfb168, cleanLineStart=@0x7fff5fbfb120, cleanLineBidiStatus=@0x7fff5fbfb108, consecutiveHyphenatedLines=0) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlockLineLayout.cpp:1240
#27 0x000000010245efcc in WebCore::RenderBlock::layoutRunsAndFloats (this=0x108e44c58, layoutState=@0x7fff5fbfb480, hasInlineChild=true) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlockLineLayout.cpp:1206
#28 0x0000000102465d8a in WebCore::RenderBlock::layoutInlineChildren (this=0x108e44c58, relayoutChildren=false, repaintLogicalTop=@0x7fff5fbfb76c, repaintLogicalBottom=@0x7fff5fbfb768) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlockLineLayout.cpp:1497
#29 0x000000010241743c in WebCore::RenderBlock::layoutBlock (this=0x108e44c58, relayoutChildren=false, pageLogicalHeight=0, layoutPass=WebCore::RenderBlock::NormalLayoutPass) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlock.cpp:1308
#30 0x0000000102416b07 in WebCore::RenderBlock::layout (this=0x108e44c58) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlock.cpp:1184
#31 0x000000010241fa9a in WebCore::RenderBlock::layoutBlockChild (this=0x108e2a218, child=0x108e44c58, marginInfo=@0x7fff5fbfbac0, previousFloatLogicalBottom=@0x7fff5fbfbab4, maxFloatLogicalBottom=@0x7fff5fbfbdc4) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlock.cpp:2090
#32 0x00000001024197b3 in WebCore::RenderBlock::layoutBlockChildren (this=0x108e2a218, relayoutChildren=false, maxFloatLogicalBottom=@0x7fff5fbfbdc4) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlock.cpp:2027
#33 0x000000010241745c in WebCore::RenderBlock::layoutBlock (this=0x108e2a218, relayoutChildren=false, pageLogicalHeight=0, layoutPass=WebCore::RenderBlock::NormalLayoutPass) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlock.cpp:1310
#34 0x0000000102416b07 in WebCore::RenderBlock::layout (this=0x108e2a218) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlock.cpp:1184
#35 0x000000010241fa9a in WebCore::RenderBlock::layoutBlockChild (this=0x106d30da8, child=0x108e2a218, marginInfo=@0x7fff5fbfc120, previousFloatLogicalBottom=@0x7fff5fbfc114, maxFloatLogicalBottom=@0x7fff5fbfc424) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlock.cpp:2090
#36 0x00000001024197b3 in WebCore::RenderBlock::layoutBlockChildren (this=0x106d30da8, relayoutChildren=false, maxFloatLogicalBottom=@0x7fff5fbfc424) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlock.cpp:2027
#37 0x000000010241745c in WebCore::RenderBlock::layoutBlock (this=0x106d30da8, relayoutChildren=false, pageLogicalHeight=0, layoutPass=WebCore::RenderBlock::NormalLayoutPass) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlock.cpp:1310
#38 0x0000000102416b07 in WebCore::RenderBlock::layout (this=0x106d30da8) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlock.cpp:1184
#39 0x000000010241fa9a in WebCore::RenderBlock::layoutBlockChild (this=0x106d1bce8, child=0x106d30da8, marginInfo=@0x7fff5fbfc780, previousFloatLogicalBottom=@0x7fff5fbfc774, maxFloatLogicalBottom=@0x7fff5fbfca84) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlock.cpp:2090
#40 0x00000001024197b3 in WebCore::RenderBlock::layoutBlockChildren (this=0x106d1bce8, relayoutChildren=false, maxFloatLogicalBottom=@0x7fff5fbfca84) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlock.cpp:2027
#41 0x000000010241745c in WebCore::RenderBlock::layoutBlock (this=0x106d1bce8, relayoutChildren=false, pageLogicalHeight=0, layoutPass=WebCore::RenderBlock::NormalLayoutPass) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlock.cpp:1310
#42 0x0000000102416b07 in WebCore::RenderBlock::layout (this=0x106d1bce8) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderBlock.cpp:1184
#43 0x000000010263acf5 in WebCore::RenderView::layout (this=0x106d1bce8) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/rendering/RenderView.cpp:136
#44 0x00000001019e2b70 in WebCore::FrameView::layout (this=0x1089404c0, allowSubtree=true) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/page/FrameView.cpp:1125
#45 0x00000001019ea865 in WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive (this=0x1089404c0) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/page/FrameView.cpp:2974
#46 0x0000000100da6360 in -[WebHTMLView(WebInternal) _web_updateLayoutAndStyleIfNeededRecursive] (self=0x10893f750, _cmd=0x7fff8be6e263) at /Users/bashi/c/src/third_party/WebKit/Source/WebKit/mac/WebView/WebHTMLView.mm:5393
#47 0x0000000100d8f322 in -[WebHTMLView(WebPrivate) viewWillDraw] (self=0x10893f750, _cmd=0x7fff835f0e1e) at /Users/bashi/c/src/third_party/WebKit/Source/WebKit/mac/WebView/WebHTMLView.mm:1271
#48 0x00007fff82d37c11 in -[NSView viewWillDraw] ()
#49 0x00007fff82d37c11 in -[NSView viewWillDraw] ()
#50 0x00007fff82d384a2 in -[NSScrollView viewWillDraw] ()
#51 0x00007fff82d37c11 in -[NSView viewWillDraw] ()
#52 0x00007fff82d37c11 in -[NSView viewWillDraw] ()
#53 0x00007fff82d37c11 in -[NSView viewWillDraw] ()
#54 0x00007fff82d37c11 in -[NSView viewWillDraw] ()
#55 0x00007fff82d36952 in -[NSView _sendViewWillDrawInRect:clipRootView:suppressRecursion:] ()
#56 0x00007fff82d356c1 in -[NSView displayIfNeeded] ()
#57 0x0000000100023ca5 in -[FrameLoadDelegate webView:didFinishLoadForFrame:] (self=0x10b20f9e0, _cmd=0x7fff835ca473, sender=0x108913960, frame=0x108921660) at /Users/bashi/c/src/third_party/WebKit/Tools/DumpRenderTree/mac/FrameLoadDelegate.mm:228
#58 0x0000000100d2ccdc in CallDelegate (implementation=0x100023ac0 <-[FrameLoadDelegate webView:didFinishLoadForFrame:]>, self=0x108913960, delegate=0x10b20f9e0, selector=0x7fff835ca473, object=0x108921660) at /Users/bashi/c/src/third_party/WebKit/Source/WebKit/mac/WebView/WebDelegateImplementationCaching.mm:285
#59 0x0000000100d2cc1d in CallFrameLoadDelegate (implementation=0x100023ac0 <-[FrameLoadDelegate webView:didFinishLoadForFrame:]>, self=0x108913960, selector=0x7fff835ca473, object=0x108921660) at /Users/bashi/c/src/third_party/WebKit/Source/WebKit/mac/WebView/WebDelegateImplementationCaching.mm:504
#60 0x0000000100d54c76 in WebFrameLoaderClient::dispatchDidFinishLoad (this=0x108921c60) at /Users/bashi/c/src/third_party/WebKit/Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:654
#61 0x00000001019bf915 in WebCore::FrameLoader::checkLoadCompleteForThisFrame (this=0x1080448d8) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/loader/FrameLoader.cpp:2282
#62 0x00000001019b7123 in WebCore::FrameLoader::checkLoadComplete (this=0x1080448d8) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/loader/FrameLoader.cpp:2457
#63 0x00000001017602f7 in WebCore::DocumentLoader::removeSubresourceLoader (this=0x10804d000, loader=0x107858a00) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/loader/DocumentLoader.cpp:776
#64 0x000000010279cf01 in WebCore::SubresourceLoader::releaseResources (this=0x107858a00) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/loader/SubresourceLoader.cpp:313
#65 0x0000000102667665 in WebCore::ResourceLoader::didFinishLoading (this=0x107858a00, finishTime=0) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/loader/ResourceLoader.cpp:313
#66 0x000000010279c9ec in WebCore::SubresourceLoader::didFinishLoading (this=0x107858a00, finishTime=0) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/loader/SubresourceLoader.cpp:268
#67 0x0000000102667edc in WebCore::ResourceLoader::didFinishLoading (this=0x107858a00, finishTime=0) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/loader/ResourceLoader.cpp:451
#68 0x0000000102664805 in -[WebCoreResourceHandleAsDelegate connectionDidFinishLoading:] (self=0x106d2b7a0, _cmd=0x7fff888b9916, connection=0x106d28200) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/platform/network/mac/ResourceHandleMac.mm:886
#69 0x00007fff8e731712 in ___NSURLConnectionDidFinishLoading_block_invoke_1 ()
#70 0x00007fff8e731692 in _NSURLConnectionDidFinishLoading ()
#71 0x00007fff8b64eee2 in URLConnectionClient::_clientDidFinishLoading ()
#72 0x00007fff8b6fed0e in URLConnectionClient::ClientConnectionEventQueue::processAllEventsAndConsumePayload ()
#73 0x00007fff8b629dfd in URLConnectionClient::processEvents ()
#74 0x00007fff8b629ca2 in MultiplexerSource::perform ()
#75 0x00007fff8828ab51 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ ()
#76 0x00007fff8828a3bd in __CFRunLoopDoSources0 ()
#77 0x00007fff882b11a9 in __CFRunLoopRun ()
#78 0x00007fff882b0ae6 in CFRunLoopRunSpecific ()
#79 0x00007fff8e6d504f in -[NSRunLoop(NSRunLoop) runMode:beforeDate:] ()
#80 0x00000001000164be in runTest (testPathOrURL=@0x7fff5fbff7a8) at /Users/bashi/c/src/third_party/WebKit/Tools/DumpRenderTree/mac/DumpRenderTree.mm:1332
#81 0x0000000100015205 in dumpRenderTree (argc=3, argv=0x7fff5fbff830) at /Users/bashi/c/src/third_party/WebKit/Tools/DumpRenderTree/mac/DumpRenderTree.mm:859
#82 0x0000000100016c4c in main (argc=3, argv=0x7fff5fbff830) at /Users/bashi/c/src/third_party/WebKit/Tools/DumpRenderTree/mac/DumpRenderTree.mm:893
(gdb) f 12
#12 0x0000000102730efe in WebCore::SimpleFontData::canRenderCombiningCharacterSequence (this=0x10782d600, characters=0x108e56a40, length=3) at /Users/bashi/c/src/third_party/WebKit/Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm:451
451        RetainPtr<CTLineRef> line(AdoptCF, wkCreateCTLineWithUniCharProvider(&provideStringAndAttributes, 0, &info));
(gdb) p info
$5 = {
  characters = 0x108e56a40, 
  length = 3, 
  attributes = 0x108985a70
}
(gdb) p/x *characters@length
$6 = {0x845b, 0xdb40, 0xdd00}
------- Comment #5 From 2012-01-15 16:24:15 PST -------
Hi mitz,

Do you have any ideas about the crash? I'd like to fix the problem.
------- Comment #6 From 2012-01-18 17:08:58 PST -------
Hi mitz,

Please let me know if you are not the right person to ask.

(In reply to comment #5)
> Hi mitz,
> 
> Do you have any ideas about the crash? I'd like to fix the problem.
------- Comment #7 From 2012-01-19 00:37:43 PST -------
Can you attach the crash log file from ~/Library/Logs/DiagnosticMessages?
------- Comment #8 From 2012-01-19 00:43:32 PST -------
Created an attachment (id=123087) [details]
crash log
------- Comment #9 From 2012-01-19 11:34:02 PST -------
(In reply to comment #8)
> Created an attachment (id=123087) [details] [details]
> crash log

Thank you!
------- Comment #10 From 2012-01-20 09:08:42 PST -------
Created an attachment (id=123336) [details]
Patched font

The crash is caused by a Core Text issue, which Apple is tracking internally. Meanwhile, I am attaching a modified version of gw432047.ttf that works around the crash, but otherwise behaves identically.
------- Comment #11 From 2012-01-22 17:03:25 PST -------
Created an attachment (id=123502) [details]
Patch (patched font added)
------- Comment #12 From 2012-01-22 17:05:34 PST -------
(In reply to comment #10)
> The crash is caused by a Core Text issue, which Apple is tracking internally. Meanwhile, I am attaching a modified version of gw432047.ttf that works around the crash, but otherwise behaves identically.

Thank you so much for your help! I checked the font works around the crash and DRT passes fast/text/unicode-variation-selector.html.

Mitz, could you please review the patch?
------- Comment #13 From 2012-01-24 13:24:04 PST -------
(From update of attachment 123502 [details])
Thanks for the patch. All variation selectors are combining marks, so there is no need to treat them specially. In particular, advanceByCombiningCharacterSequence() already handles to BMP variation selectors (U+180B, U+180C, U+180D, U+FE00…U+FE0F) correctly. It is just not set up to handle non-BMP marks. You should change the loop in advanceByCombiningCharacterSequence() so that it gets and iterates by full Unicode character instead of UTF-16 code unit.
------- Comment #14 From 2012-01-24 20:47:39 PST -------
Created an attachment (id=123878) [details]
Patch
------- Comment #15 From 2012-01-24 20:49:23 PST -------
(In reply to comment #13)
> (From update of attachment 123502 [details] [details])
> Thanks for the patch. All variation selectors are combining marks, so there is no need to treat them specially. In particular, advanceByCombiningCharacterSequence() already handles to BMP variation selectors (U+180B, U+180C, U+180D, U+FE00…U+FE0F) correctly. It is just not set up to handle non-BMP marks. You should change the loop in advanceByCombiningCharacterSequence() so that it gets and iterates by full Unicode character instead of UTF-16 code unit.

Updated the patch. Thank you for review and detailed suggestion!
------- Comment #16 From 2012-01-24 21:14:14 PST -------
(From update of attachment 123878 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=123878&action=review

> Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:217
> +        if (U16_IS_SURROGATE(nextCharacter)) {
> +            if (!U16_IS_LEAD(nextCharacter) || markIterator == end)
> +                return false;
> +            UChar trail = *markIterator++;
> +            if (!U16_IS_TRAIL(trail))
> +                return false;
> +            nextCharacter = U16_GET_SUPPLEMENTARY(nextCharacter, trail);
> +        }

I think this can be written more succinctly, as long as you’re adding a local variable, using U16_NEXT.

> Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:220
>          markCount++;

I think despite markCount’s misleading name, if you look at how it’s used, you’ll see that it should be incremented by the number of UChars, i.e. 2 in the surrogate-pair case.
------- Comment #17 From 2012-01-24 22:29:17 PST -------
Created an attachment (id=123885) [details]
Patch
------- Comment #18 From 2012-01-24 22:30:33 PST -------
(From update of attachment 123878 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=123878&action=review

>> Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:217
>> +        }
> 
> I think this can be written more succinctly, as long as you’re adding a local variable, using U16_NEXT.

Modified to use U16_NEXT.

>> Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:220
>>          markCount++;
> 
> I think despite markCount’s misleading name, if you look at how it’s used, you’ll see that it should be incremented by the number of UChars, i.e. 2 in the surrogate-pair case.

I misunderstood how it is used. Thank you for correction.
------- Comment #19 From 2012-01-25 04:33:05 PST -------
(From update of attachment 123885 [details])
Clearing flags on attachment: 123885

Committed r105866: <http://trac.webkit.org/changeset/105866>
------- Comment #20 From 2012-01-25 04:33:10 PST -------
All reviewed patches have been landed.  Closing bug.