[webkit-reviews] review granted: [Bug 90626] CSS 2.1 failure: vertical-align-boxes-001 fails : [Attachment 150971] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Robert Hogan <robert at webkit.org>'s
request for review:
Bug 90626: CSS 2.1 failure: vertical-align-boxes-001 fails
https://bugs.webkit.org/show_bug.cgi?id=90626

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

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=150971&action=review


> Source/WebCore/rendering/RootInlineBox.cpp:890
> +	       if (renderer->style()->verticalAlignLength().isPercent())
> +		   verticalPosition -=
valueForLength(renderer->style()->verticalAlignLength(),
renderer->style()->computedLineHeight(), renderer->view());
> +	       else
> +		   verticalPosition -=
valueForLength(renderer->style()->verticalAlignLength(),
renderer->lineHeight(firstLine, lineDirection), renderer->view());

I would have used a local to store the result of either
"renderer->style()->computedLineHeight()" or "renderer->lineHeight(firstLine,
lineDirection)", means slightly less copy/paste code.

Also, can you please add a comment (link to the spec?) explaining why this is
done.  It don't think it will be obvious to future readers of this code.