WebKit uses wrong baseline for empty inline-flex containers, when doing `vertical-align:baseline` alignment

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Daniel Holbert]

(Note: there's a Blink version of this bug at https://bugs.chromium.org/p/chromium/issues/detail?id=946669 )

STR:
(1) View this fiddle: https://jsfiddle.net/dholbert/y6xef59s/

EXPECTED RESULTS:
The four boxes should all look the same, with "abc" aligned consistently at the margin-bottom of the black-bordered area (it should be aligned 10px below the black border).

ACTUAL RESULTS:
"abc" is aligned further up, in the second case (for inline-flex). It looks like the bottom of the black-bordered-area's content-box is being used for baseline-alignment, rather than the bottom of its margin-box.



BROWSER COMPARISON:
Firefox 66 and Edge 18 both give "expected results"
Safari Safari 12 gives "actual results"
Chrome gives a more-severe version of "actual results" -- they have "abc" misaligned in the 3rd box as well (the inline-grid).


SPEC NOTES/JUSTIFICATION:
The alignment in play here is `vertical-align:baseline`, which is specced as follows (emphasis added)
 # [...] If the box does not have a baseline,
 # align the **bottom margin edge**
 # with the parent's baseline.
https://drafts.csswg.org/css2/visudet.html#propdef-vertical-align

That should apply here, because an empty flex container does not have a baseline, as noted here:

 # [...] if the flex container has at least one item [...]
 # Otherwise, the flex container **has no first/last main-axis baseline set**
 # and one is synthesized if needed according to the rules of its alignment context.
https://drafts.csswg.org/css-flexbox-1/#flex-baselines

[Manuel Rego Casasnovas]

This has been recently fixed in Blink:
https://chromium-review.googlesource.com/c/chromium/src/+/1533952

So maybe we could just port that change into WebKit.

[Radar WebKit Bug Importer]

<rdar://problem/49358608>

[Manuel Rego Casasnovas]

Created attachment 366807 [details]
Patch

[Javier Fernandez]

Please, fix the layout tests failures before landing. I guess you'd need to extract the new baseline from the specific bots for the ref tests using images.

[Manuel Rego Casasnovas]

(In reply to Javier Fernandez from comment #4)
> Please, fix the layout tests failures before landing. I guess you'd need to
> extract the new baseline from the specific bots for the ref tests using
> images.

Yes sure, that's why it was not marked with "r?" yet, as I uploaded to get the new baselines from EWSs.

Thanks for the review anyway.

[EWS Watchlist]

Comment on attachment 366807 [details]
Patch

Attachment 366807 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11777553

New failing tests:
svg/custom/foreign-object-skew.svg
fast/forms/select-baseline.html
fast/forms/button-generated-content.html
css3/flexbox/flexbox-baseline-margins.html
imported/blink/fast/forms/button-baseline-and-collapsing.html
css3/flexbox/button.html

[EWS Watchlist]

Created attachment 366809 [details]
Archive of layout-test-results from ews100 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 366807 [details]
Patch

Attachment 366807 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11777584

New failing tests:
svg/custom/foreign-object-skew.svg
fast/forms/select-baseline.html
fast/forms/button-generated-content.html
css3/flexbox/flexbox-baseline-margins.html
imported/blink/fast/forms/button-baseline-and-collapsing.html
css3/flexbox/button.html

[EWS Watchlist]

Created attachment 366810 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 366807 [details]
Patch

Attachment 366807 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11777639

New failing tests:
svg/custom/foreign-object-skew.svg
fast/forms/select-baseline.html
fast/forms/button-generated-content.html
css3/flexbox/flexbox-baseline-margins.html
imported/blink/fast/forms/button-baseline-and-collapsing.html
css3/flexbox/button.html

[EWS Watchlist]

Created attachment 366811 [details]
Archive of layout-test-results from ews117 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 366807 [details]
Patch

Attachment 366807 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11777674

New failing tests:
svg/custom/foreign-object-skew.svg
fast/forms/select-baseline.html
fast/forms/button-generated-content.html
css3/flexbox/flexbox-baseline-margins.html
imported/blink/fast/forms/button-baseline-and-collapsing.html
css3/flexbox/button.html

[EWS Watchlist]

Created attachment 366812 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 366807 [details]
Patch

Attachment 366807 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11779967

New failing tests:
fast/loader/charset-parse.html
fast/loader/create-frame-in-DOMContentLoaded.html
fast/forms/select-baseline.html
fast/loader/crash-copying-backforwardlist.html
fast/forms/button-generated-content.html
fast/loader/comment-only-javascript-url.html
css3/flexbox/flexbox-baseline-margins.html
security/contentSecurityPolicy/video-with-file-url-allowed-by-media-src-star.html
imported/blink/fast/forms/button-baseline-and-collapsing.html
fast/loader/crash-replacing-location-before-load.html

[EWS Watchlist]

Created attachment 366822 [details]
Archive of layout-test-results from ews201 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews201  Port: win-future  Platform: CYGWIN_NT-10.0-2.11.1-0.329-5-3-x86_64-64bit

[Manuel Rego Casasnovas]

Created attachment 367116 [details]
Patch

[EWS Watchlist]

Comment on attachment 367116 [details]
Patch

Attachment 367116 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11829026

New failing tests:
svg/custom/foreign-object-skew.svg
imported/blink/fast/forms/button-baseline-and-collapsing.html
fast/forms/select-baseline.html
css3/flexbox/button.html
fast/forms/button-generated-content.html

[EWS Watchlist]

Created attachment 367117 [details]
Archive of layout-test-results from ews102 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 367116 [details]
Patch

Attachment 367116 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11829049

New failing tests:
svg/custom/foreign-object-skew.svg
imported/blink/fast/forms/button-baseline-and-collapsing.html
fast/forms/select-baseline.html
css3/flexbox/button.html
fast/forms/button-generated-content.html

[EWS Watchlist]

Created attachment 367118 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 367116 [details]
Patch

Attachment 367116 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11829087

New failing tests:
svg/custom/foreign-object-skew.svg
imported/blink/fast/forms/button-baseline-and-collapsing.html
fast/forms/select-baseline.html
css3/flexbox/button.html
fast/forms/button-generated-content.html

[EWS Watchlist]

Created attachment 367119 [details]
Archive of layout-test-results from ews117 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 367116 [details]
Patch

Attachment 367116 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11829092

New failing tests:
svg/custom/foreign-object-skew.svg
imported/blink/fast/forms/button-baseline-and-collapsing.html

[EWS Watchlist]

Created attachment 367120 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 367116 [details]
Patch

Attachment 367116 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11829275

New failing tests:
imported/blink/fast/forms/button-baseline-and-collapsing.html

[EWS Watchlist]

Created attachment 367125 [details]
Archive of layout-test-results from ews204 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews204  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit

[Manuel Rego Casasnovas]

Created attachment 367304 [details]
Patch

[Manuel Rego Casasnovas]

Comment on attachment 367304 [details]
Patch

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

> Source/WebCore/rendering/RenderButton.h:64
> +    int baselinePosition(FontBaseline, bool firstLine, LineDirectionMode, LinePositionMode = PositionOnContainingLine) const override;

Javi could you please take a new look?

I needed to do this in order to keep baseline behavior for RenderButton.
The fact that a <button> is a flexbox is an implementation detail, but the special conditions of flexbox baseline shouldn't apply to buttons.
Actually in Blink this is also overridden, but that was there before the patch I'm porting, that's why I didn't realized before.

[WebKit Commit Bot]

Comment on attachment 367304 [details]
Patch

Clearing flags on attachment: 367304

Committed r244213: <https://trac.webkit.org/changeset/244213>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.