Bug 200675 – Rename StringBuilder::append(UChar32) to StringBuilder::appendCharacter(UChar32) to avoid accidental change in behavior when replacing append with flexibleAppend

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sam Weinig]

Rename StringBuilder::append(UChar32) to StringBuilder::appendCharacter(SUChar32) to avoid accidental change in behavior when replacing append with flexibleAppend

[Sam Weinig]

When we switch StringBuilder::append(...) to be based on the StringConcatenate/makeString flexibleAppend implementation, if we don't change anything, the behavior of StringBuilder::append(UChar32) will go from appending a character to appending a stringified number.

To work around this, we can rename StringBuilder::append(UChar32) to StringBuilder::appendCharacter(UChar32) and update all the call sites.

[Sam Weinig]

Longer term, it would be nicer to have a better solution for this, and one that can be used consistently between makeString() and StringBuilder.

The only idea I have right now is adding a struct Character/CodePoint { UChar32 } that people can wrap UChar32s in.

[Sam Weinig]

Created attachment 376174 [details]
Patch

[Darin Adler]

Comment on attachment 376174 [details]
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.

[Darin Adler]

Comment on attachment 376174 [details]
Patch

Another thought: We might want at least one overload  of appendCharacter even if they are deleted so we don’t call this slightly more expensive function accidentally.  For  types like char, UChar and LChar.

[Sam Weinig]

Committed r248659: <https://trac.webkit.org/changeset/248659>

[Radar WebKit Bug Importer]

<rdar://problem/54286358>