[webkit-reviews] review granted: [Bug 102795] [css3-text] Add rendering support for -webkit-text-underline-position : [Attachment 193309] css3-text-underline-position

[bugzilla-daemon at webkit.org]

Levi Weintraub <leviw at chromium.org> has granted Lamarque V. Souza
<Lamarque.Souza at basyskom.com>'s request for review:
Bug 102795: [css3-text] Add rendering support for
-webkit-text-underline-position
https://bugs.webkit.org/show_bug.cgi?id=102795

Attachment 193309: css3-text-underline-position
https://bugs.webkit.org/attachment.cgi?id=193309&action=review

------- Additional Comments from Levi Weintraub <leviw at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=193309&action=review


The pixel tests make me sad, but it seems like they're necessary.

>
LayoutTests/fast/css3-text/css3-text-decoration/text-underline-position/text-un
derline-position-all.html:4
> +    <!-- Bugzilla link: http://webkit.org/b/102795 -->

I'd rather see this in the description than in a comment in the test.

>
LayoutTests/fast/css3-text/css3-text-decoration/text-underline-position/text-un
derline-position-alphabetic.html:4
> +    <!-- Bugzilla link: http://webkit.org/b/102795 -->

Ditto.

>
LayoutTests/fast/css3-text/css3-text-decoration/text-underline-position/text-un
derline-position-auto.html:4
> +    <!-- Bugzilla link: http://webkit.org/b/102795 -->

Ditto.

>
LayoutTests/fast/css3-text/css3-text-decoration/text-underline-position/text-un
derline-position-under-out-of-flow.html:4
> +    <!-- Bugzilla link: http://webkit.org/b/102795 -->

Ditto.

>
LayoutTests/fast/css3-text/css3-text-decoration/text-underline-position/text-un
derline-position-under-out-of-flow.html:12
> +    <p>Test if out of flow boxes are ignored when calculating the offset for
'text-underline-position: under'. The horizontal line below should not go
outside the box.</p>

Nit observation: Strangely long line when you wrapped the other descriptions.