[Webkit-unassigned] [Bug 77067] Parenthesis in RTL fonts are not mirrored with SVG fonts

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #124330|review?                     |review-
               Flag|                            |




--- Comment #13 from Darin Adler <darin at apple.com>  2012-01-27 10:42:14 PST ---
(From update of attachment 124330)
View in context: https://bugs.webkit.org/attachment.cgi?id=124330&action=review

> Source/WebCore/svg/SVGFontData.cpp:279
> +    StringBuilder mirroredCharacters;
> +    mirroredCharacters.reserveCapacity(length);
> +
> +    for (unsigned i = 0; i < length; ++i)
> +        mirroredCharacters.append((UChar) mirroredChar(characters[i]));
> +
> +    return mirroredCharacters.toString();

This loop is incorrect. It walks through UTF-16 character codes rather than walking through Unicode characters so won’t correctly mirror non-BMP characters. The need to cast mirroredChar’s result to (UChar) emphasizes the mistake.

It’s straightforward to iterate the characters instead and handle surrogates correctly. There are examples of the use of U16_NEXT to read a character at a time in functions such as SegmentedFontData::containsCharacters and we can correctly write the characters in UTF-16 by using U16_LENGTH, U16_LEAD, and U16_TRAIL.

> Source/WebCore/svg/SVGFontData.h:59
> +    String replaceMirroredCharacters(const UChar* characters, unsigned length) const;

Naming this function that returns a newly created string “replace” is not so great. I would name it something like createStringWithMirroredCharacters.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.