[Webkit-unassigned] [Bug 16131] ZWNJ - Display non-printing, invisible character

[bugzilla-daemon at webkit.org]

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


mitz at webkit.org changed:

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




--- Comment #15 from mitz at webkit.org  2010-03-19 09:38:17 PST ---
(From update of attachment 51156)
This patch consist mostly of whitespace and coding style changes. It’s okay to
make such changes in a function you’re already changing as part of the bugfix,
but otherwise please keep large-scale changes out of a bugfix patch.

> +2010-03-19  David Yonge-Mallo  <davinci at chromium.org>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Fix the (non)display of glyphs for ZWJ and ZWNJ in simple font code path.
> +        Added logic to use complex font code path for Arabic presentation forms.
> +
> +        https://bugs.webkit.org/show_bug.cgi?id=16131
> +
> +        Tests: fast/text/format-control.html
> +               fast/text/zero-width-characters.html
> +               fast/text/international/zero-width-joiner-and-non-joiner.html
> +
> +        * platform/graphics/Font.h:
> +        (WebCore::Font::treatAsZeroWidthSpace): treat ZWJ and ZWNJ as ZWSP.
> +        * platform/graphics/FontFastPath.cpp:
> +        (WebCore::Font::canUseGlyphCache): added Arabic presentation forms
> +        * platform/graphics/WidthIterator.cpp:
> +        (WebCore::WidthIterator::advance): don't display glyphs for ZWJ and ZWNJ.

This change log entry is outdated.


> -        return (((c & ~0xFF) == 0 && gRoundingHackCharacterTable[c]));
> +        return (( !(c & ~0xFF) && gRoundingHackCharacterTable[c]));

This is an unrelated style change, so like I said, it should not be included in
this patch, but I can’t help pointing out that it leaves the redundant
parentheses in place and adds a space before the ! that shouldn’t be there.


> +                    buffer[zeroWidthNonJoiner - start] = zeroWidthSpace;
> +                    buffer[zeroWidthJoiner - start] = zeroWidthSpace;

This is good.

>              UErrorCode uStatus = U_ZERO_ERROR;  
>              int32_t resultLength = unorm_normalize(m_run.data(currentCharacter), 2,
>                  UNORM_NFC, UNORM_UNICODE_3_2, &normalizedCharacters[0], 2, &uStatus);
> -            if (resultLength == 1 && uStatus == 0)
> +            if (resultLength == 1 && !uStatus)

It’s true that WebKit style is to use ! instead of == 0, but in this case, I
think == U_ZERO_ERROR is even more correct.

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