[Webkit-unassigned] [Bug 231606] ASSERT hit in surrogatePairAwareIndex and surrogatePairAwareStart lambdas for text with unpaired surrogates

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=231606

--- Comment #20 from Darin Adler <darin at apple.com> ---
Comment on attachment 441150
  --> https://bugs.webkit.org/attachment.cgi?id=441150
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=441150&action=review

Thanks for taking advantage of my suggestion to use higher level U16 macros instead of writing the code ourselves. I think it’s looking good.

> Source/WebCore/layout/formattingContexts/inline/text/TextUtil.cpp:149
> +        U16_SET_CP_LIMIT(text, 0, index, text.length());

I think it’s a little bit less clear to modify an argument, index, than to modify a copy of it. It’s an informal coding style rule in WebKit that we typically don’t use arguments that way, even when they are passed by value and can be used that way.

> Source/WebCore/layout/formattingContexts/inline/text/TextUtil.cpp:159
> +    auto right = surrogatePairAwareIndex(std::min<unsigned>(left + offset + 1, (startPosition + length)));

Why add the + 1 for the the argument to surrogatePairAwareIndex at each place it’s called? Seems like the + 1 could be done inside that function.

Maybe you think the function makes more sense with these semantics? If so, that’s OK, but if not, I’d do it there.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20211013/f087d8ef/attachment.htm>