Bug 96188

Summary: inline-flex baseline is sometimes wrong
Product: WebKit Reporter: Dave Barton <dbarton>
Component: Layout and RenderingAssignee: Tony Chang <tony>
Status: RESOLVED FIXED    
Severity: Normal CC: davidc, eric, fred.wang, gyuyoung.kim, ojan, rakuco, shezbaig.wk, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 95889    
Attachments:
Description Flags
inconsistent inline-flex baselines
none
column, baseline, tall really strange
none
Patch
none
Patch
none
Patch for landing webkit.review.bot: commit-queue-

Dave Barton
Reported 2012-09-08 18:07:06 PDT
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!
Attachments
inconsistent inline-flex baselines (1.15 KB, text/html)
2012-09-08 18:07 PDT, Dave Barton 		no flags
Details
column, baseline, tall really strange (1.13 KB, text/html)
2012-09-09 12:07 PDT, Dave Barton 		no flags
Details
Patch (76.80 KB, patch)
2012-10-03 15:09 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
Patch (78.69 KB, patch)
2012-10-03 15:33 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
Patch for landing (85.46 KB, patch)
2012-10-03 16:53 PDT, Tony Chang 		webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Dave Barton
Comment 1 2012-09-09 12:07:36 PDT
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
Comment 2 2012-09-10 16:20:26 PDT
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
Comment 3 2012-09-28 16:00:16 PDT
The second test case is a bug in align-items. I've filed bug 97948 for it.
Tony Chang
Comment 4 2012-10-03 15:09:42 PDT
Created attachment 166968 [details] Patch
Tony Chang
Comment 5 2012-10-03 15:25:48 PDT
Comment on attachment 166968 [details] Patch Actually, I'm going to change one of the results.
Tony Chang
Comment 6 2012-10-03 15:33:28 PDT
Created attachment 166972 [details] Patch
WebKit Review Bot
Comment 7 2012-10-03 15:35:42 PDT
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 8 2012-10-03 16:14:38 PDT
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
Comment 9 2012-10-03 16:53:05 PDT
Created attachment 166996 [details] Patch for landing
WebKit Review Bot
Comment 10 2012-10-03 19:58:56 PDT
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
Comment 11 2012-10-04 09:47:36 PDT
Committed r130405: <http://trac.webkit.org/changeset/130405>
Note You need to log in before you can comment on or make changes to this bug.