[webkit-reviews] review granted: [Bug 63319] Refactor text iterator code respecting surrogate pairs from WidthIterator : [Attachment 98484] Patch v3

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has granted Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 63319: Refactor text iterator code respecting surrogate pairs from
WidthIterator
https://bugs.webkit.org/show_bug.cgi?id=63319

Attachment 98484: Patch v3
https://bugs.webkit.org/attachment.cgi?id=98484&action=review

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=98484&action=review

The patch looks good. I wonder about the failing tests on chromium. I hope you
run the complete DRT suite. r=me if it passes all tests on your machine.

>> Source/WebCore/platform/graphics/SurrogatePairAwareTextIterator.cpp:72
>> +	// Do we have a surrogate pair?  If so, determine the full Unicode (32
bit) code point before glyph lookup.
> 
> Should have only a single space after a punctuation in a comment. 
[whitespace/comments] [5]

Thats right :-) Can you fix it before landing?

>> Source/WebCore/platform/graphics/SurrogatePairAwareTextIterator.cpp:106
>> +	    if (resultLength == 1 && uStatus == 0)
> 
> Tests for true/false, null/non-null, and zero/non-zero should all be done
without equality comparisons.  [readability/comparison_to_zero] [5]

Should also be a simple fix