[webkit-reviews] review granted: [Bug 209692] [Cocoa] Password obscuring dots drawn with the system font are too small : [Attachment 395874] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 209692: [Cocoa] Password obscuring dots drawn with the system font are too
small
https://bugs.webkit.org/show_bug.cgi?id=209692

Attachment 395874: Patch

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




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

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

> Source/WebCore/rendering/RenderBlock.cpp:3520
> +    if (style.textSecurity() != TextSecurity::Disc)

For iOS family, should run the code below for TextSecurity::Square and
TextSecurity::Circle.

It would be safe, because it will never transform any character other than
blackCircle. So it won’t ever change a square or hollow circle.

If would be helpful and effective, because otherwise the blackCircle character
will be used for TextSecurity::Square and TextSecurity::Circle.

So I think we should consider changing this.

> Source/WebCore/rendering/RenderBlock.cpp:3533
> +#if PLATFORM(IOS_FAMILY)
> +    constexpr UChar discCharacterToReplace = blackCircle;
> +#else
> +    constexpr UChar discCharacterToReplace = bullet;
> +#endif

It’s irritating to have the connection between the code here and the code in
RenderText::setRenderedText, with no comment there pointing here.

I feel like it’s my fault since I encouraged you to put the code here instead
of there. Can live with it for now, I guess.