[webkit-reviews] review granted: [Bug 195125] [iOS] Caret x-position in empty text area does not match text field : [Attachment 363370] Patch and layout tests

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Daniel Bates <dbates at webkit.org>'s
request for review:
Bug 195125: [iOS] Caret x-position in empty text area does not match text field
https://bugs.webkit.org/show_bug.cgi?id=195125

Attachment 363370: Patch and layout tests

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




--- Comment #7 from Darin Adler <darin at apple.com> ---
Comment on attachment 363370
  --> https://bugs.webkit.org/attachment.cgi?id=363370
Patch and layout tests

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

> Source/WebCore/css/html.css:400
> +    padding: 0.2em 0.5em 0.3em 0.5em; /* Keep padding-left, padding-right in
sync with textarea selector (below). */

Should say "keep the values the same", not "keep them in sync" since "in sync"
sounds like something more sophisticated that could mean something different.
Also, this comment, like all comments, ought to say *why*, not just leave
instructions for future programmers to follow without rationale. Still want to
keep the comment wording short, if possible.

> Source/WebCore/css/html.css:681
> +    /* Keep padding-left, padding-right sync with input selector (above). */

Would be best if the comment format was identical to above. The other one is at
the end of a line but this is between lines. Also, this says "keep sync" rather
than "keep in sync", but should be reworded anyway.

But also, the thing above isn’t the input selector. On the iOS Family platform,
which is the only platform this code is active on, the above selector is a
selector that matches *many* things, including textarea, just like this one. So
it’s a misnomer to call the thing about the "input selector". And that
observation points to a way to avoid this problem. Instead of "padding: 2x",
which I suppose is what we want for other platforms, on the iOS Family
platforms maybe we should instead just do "padding-top: 2px; padding-bottom:
2px", then we will inherit the padding values from the earlier rule and won’t
need padding-left/padding-right values nor a comment telling us to keep things
"in sync". Still might need a comment explaining why we carefully avoid setting
padding-left and padding-right.