[webkit-reviews] review granted: [Bug 200675] Rename StringBuilder::append(UChar32) to StringBuilder::appendCharacter(UChar32) to avoid accidental change in behavior when replacing append with flexibleAppend : [Attachment 376174] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Sam Weinig <sam at webkit.org>'s request
for review:
Bug 200675: Rename StringBuilder::append(UChar32) to
StringBuilder::appendCharacter(UChar32) to avoid accidental change in behavior
when replacing append with flexibleAppend
https://bugs.webkit.org/show_bug.cgi?id=200675

Attachment 376174: Patch

https://bugs.webkit.org/attachment.cgi?id=376174&action=review




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

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

> Source/WTF/wtf/FileSystem.cpp:185
> +	   result.appendCharacter(toASCIIHexValue(inputString[i + 2],
inputString[i + 3]) << 8 | toASCIIHexValue(inputString[i + 4], inputString[i +
5]));

This looks like it’s a 4 digit hex value. No reason to convert that to UChar32
instead of UChar. Could just use a local UChar instead.

> Source/WebCore/css/CSSMarkup.cpp:101
> +	       appendTo.appendCharacter(0xfffd);

This one doesn’t need the UChar32 overload and is only getting that one because
the constant 0xfffd is int, not UChar. Could use the replacementCharacter UChar
constant from CharacterNames.h instead and then would be able to use the normal
append instead.

> Source/WebCore/platform/mock/mediasource/MockBox.cpp:54
> +	   builder.appendCharacter(array->item(i));

An 8-bit integer doesn’t need to be appended as a UChar32. The item function
returns an int, but one that’s guaranteed to be an 8-bit signed integer, I
think. Maybe we should add a typecast here instead of calling appendCharacter?

> Source/WebCore/platform/mock/mediasource/MockBox.cpp:75
> +	   builder.appendCharacter(array->item(i));

Ditto.