Bug 96188 – inline-flex baseline is sometimes wrong

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Dave Barton]

Created attachment 162972 [details]
inconsistent inline-flex baselines

The attached file shows that inline-flex elements with { align-items: baseline } seem to export flaky baselines to their containing block. I believe that "x" and "a" should always have the same baseline, according to <http://dev.w3.org/csswg/css3-flexbox/#flex-baselines>.

I am experimenting with using flex boxes for MathML, and except for this issue it looks good so far. The order, align-*, flex-direction, etc. properties would be very useful, as well as the prohibition against floats & columns. MathML doesn't need actual flexing, but does need these other features, including a precise baseline. If you know a reason flex boxes would be a bad idea for MathML, please state it, thanks!

[Dave Barton]

Created attachment 163005 [details]
column, baseline, tall really strange

Here the "1"s appear to the right of the "a b"s! This is really strange. You need -webkit-flex-direction: column; and -webkit-align-items: baseline; and the .tall style rule to see this happen.

[Tony Chang]

I don't think we have any tests for the baseline of the flexbox itself yet.  I'm marking this as part of the "fix the baseline of the flexbox" bug.

[Tony Chang]

The second test case is a bug in align-items.  I've filed bug 97948 for it.

[Tony Chang]

Created attachment 166968 [details]
Patch

[Tony Chang]

Comment on attachment 166968 [details]
Patch

Actually, I'm going to change one of the results.

[Tony Chang]

Created attachment 166972 [details]
Patch

[WebKit Review Bot]

Attachment 166972 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3..." exit_code: 1
LayoutTests/platform/qt/TestExpectations:2730:  Path does not exist.  [test/expectations] [5]
LayoutTests/platform/qt/TestExpectations:2731:  Path does not exist.  [test/expectations] [5]
Total errors found: 2 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ojan Vafai]

Comment on attachment 166972 [details]
Patch

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

Just fix the marginBefore issue. The rest looks good.

For thoroughness sake, might want to add a test to verify the following text from the spec:

When calculating the baseline according to the above rules, if the box contributing a baseline has an ‘overflow’ value that allows scrolling, the box must be treated as being in its initial scroll position for the purpose of determining its baseline.

> Source/WebCore/rendering/RenderFlexibleBox.cpp:242
> +        return baseline + marginBefore();

marginBefore is wrong here. For horizontal we need marginTop and for vertical we need marginRight.

> Source/WebCore/rendering/RenderFlexibleBox.cpp:380
> +

Extra line break?

[Tony Chang]

Created attachment 166996 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 166996 [details]
Patch for landing

Rejecting attachment 166996 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
ols/Scripts/new-run-webkit-tests', '--chromium', '--skip-failing-tests', '--no-ne..." exit_code: 10
forms/time-multiple-fields/time-multiple-fields-appearance-style.html [ ImageOnlyFailure ] 
  fast/forms/week-multiple-fields/week-multiple-fields-appearance-basic.html [ ImageOnlyFailure ] 
  fast/forms/week-multiple-fields/week-multiple-fields-appearance-pseudo-elements.html [ ImageOnlyFailure ] 
  fast/forms/week-multiple-fields/week-multiple-fields-appearance-style.html [ ImageOnlyFailure ] 


Full output: http://queues.webkit.org/results/14129905

[Tony Chang]

Committed r130405: <http://trac.webkit.org/changeset/130405>