[webkit-reviews] review granted: [Bug 63319] Refactor text iterator code respecting surrogate pairs from WidthIterator : [Attachment 98484] Patch v3
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jun 24 05:42:10 PDT 2011
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
More information about the webkit-reviews
mailing list