[Webkit-unassigned] [Bug 36419] A backslash in EUC-JP becomes to a yen sign when it is copied

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Apr 7 10:33:51 PDT 2010


Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
  Attachment #51234|review?                     |review+
               Flag|                            |

--- Comment #4 from Alexey Proskuryakov <ap at webkit.org>  2010-04-07 10:33:51 PST ---
(From update of attachment 51234)
This looks good enough for me to say r+. But it would be even better if Dan
and/or Darin could take a look, too. Some comments below.

> +(Note that this test makes sense only when you are using DumpRenderTree and dumpAsText. You will see yen signs when you are using Safari because of transcoding)

It seems that this test can be made to work in Safari with String.charCodeAt().

> +    for (TextIterator it(r, false, false, !isDisplayString); !it.atEnd(); it.advance()) {

This is really hard to read. It's a common problem in WebCore, but we are
trying to improve argument readability (usually by adding new overloaded
methods, or using named enums for argument values).

> +    bool m_isForCopying;

As you said, this name can be  confusing. I don't have a better suggestion, but
there should be at least a comment.

> +void RenderText::updateNeedsTranscode()
> +{
> +    const TextEncoding* encoding = document()->decoder() ? &document()->decoder()->encoding() : 0;
> +    m_needsTranscode = (encoding && encoding->backslashAsCurrencySymbol() != '\\');
> +}

TextResourceDecoder always has an encoding, so you could save a null check by
writing this in a different way.

Thanks for the good test coverage!

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

More information about the webkit-unassigned mailing list