Bug 90626 - CSS 2.1 failure: vertical-align-boxes-001 fails
Summary: CSS 2.1 failure: vertical-align-boxes-001 fails
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Robert Hogan
URL:
Keywords:
Depends on: 47141 91372
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-05 12:16 PDT by Robert Hogan
Modified: 2012-07-18 15:09 PDT (History)
5 users (show)

See Also:


Attachments
Patch (7.76 KB, patch)
2012-07-05 12:39 PDT, Robert Hogan
eric: review+
eric: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Hogan 2012-07-05 12:16:32 PDT
.
Comment 1 Robert Hogan 2012-07-05 12:39:25 PDT
Created attachment 150971 [details]
Patch
Comment 2 Eric Seidel (no email) 2012-07-12 13:23:52 PDT
Comment on attachment 150971 [details]
Patch

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.
Comment 3 Robert Hogan 2012-07-14 01:58:24 PDT
Committed r122663: <http://trac.webkit.org/changeset/122663>
Comment 4 Vsevolod Vlasov 2012-07-16 03:35:53 PDT
It is failing on chromium win: https://bugs.webkit.org/show_bug.cgi?id=91372
Comment 5 Jer Noble 2012-07-18 14:45:20 PDT
This looks to have broken the following tests on Lion:

mathml/presentation/mo.xhtml
mathml/presentation/row.xhtml

http://build.webkit.org/results/Apple%20Lion%20Release%20WK1%20(Tests)/r123013%20(1287)/results.html
Comment 6 Jer Noble 2012-07-18 15:09:09 PDT
https://bugs.webkit.org/show_bug.cgi?id=91677