[webkit-reviews] review granted: [Bug 16768] Position and thickness of underline should scale accordingly as font size changes : [Attachment 223908] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 16768: Position and thickness of underline should scale accordingly as font
size changes
https://bugs.webkit.org/show_bug.cgi?id=16768

Attachment 223908: Patch
https://bugs.webkit.org/attachment.cgi?id=223908&action=review

------- Additional Comments from Dean Jackson <dino at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=223908&action=review


> Source/WebCore/ChangeLog:8
> +	   This patch simply triggers the previous iOS codepath for underline
drawing code.

Maybe this should say: This patch adopts the iOS codepath for underlines on OS
X.

>
LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-thickness.html:
8
> +#t {
> +    text-decoration: underline;
> +    font-size: 10000px;
> +}

Why not just put this in the style attribute?

>
LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-thickness.html:
16
> +<div style="position: absolute; left: -3000px; top: -9650px;" id="t">T</div>


You should use Ahem font so that you can be sure the metrics are consistent.