Bug 196312 - WebKit uses wrong baseline for empty inline-flex containers, when doing `vertical-align:baseline` alignment
Summary: WebKit uses wrong baseline for empty inline-flex containers, when doing `vert...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: Safari 12
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Manuel Rego Casasnovas
URL: https://jsfiddle.net/dholbert/y6xef59s/
Keywords: BlinkMergeCandidate, InRadar
Depends on:
Blocks:
 
Reported: 2019-03-27 13:11 PDT by Daniel Holbert
Modified: 2019-04-12 05:04 PDT (History)
9 users (show)

See Also:


Attachments
Patch (145.25 KB, patch)
2019-04-05 04:09 PDT, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-highsierra (2.80 MB, application/zip)
2019-04-05 05:13 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-highsierra-wk2 (3.02 MB, application/zip)
2019-04-05 05:27 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews117 for mac-highsierra (2.61 MB, application/zip)
2019-04-05 05:55 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews125 for ios-simulator-wk2 (3.01 MB, application/zip)
2019-04-05 06:03 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews201 for win-future (13.41 MB, application/zip)
2019-04-05 10:07 PDT, Build Bot
no flags Details
Patch (884.26 KB, patch)
2019-04-10 05:56 PDT, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-highsierra (2.62 MB, application/zip)
2019-04-10 07:01 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-highsierra-wk2 (2.86 MB, application/zip)
2019-04-10 07:15 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews117 for mac-highsierra (2.47 MB, application/zip)
2019-04-10 07:46 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (2.72 MB, application/zip)
2019-04-10 07:52 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews204 for win-future (12.85 MB, application/zip)
2019-04-10 08:32 PDT, Build Bot
no flags Details
Patch (273.85 KB, patch)
2019-04-12 02:09 PDT, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Holbert 2019-03-27 13:11:07 PDT
(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
Comment 1 Manuel Rego Casasnovas 2019-03-27 14:38:44 PDT
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.
Comment 2 Radar WebKit Bug Importer 2019-03-27 16:20:36 PDT
<rdar://problem/49358608>
Comment 3 Manuel Rego Casasnovas 2019-04-05 04:09:43 PDT
Created attachment 366807 [details]
Patch
Comment 4 Javier Fernandez 2019-04-05 04:52:52 PDT
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.
Comment 5 Manuel Rego Casasnovas 2019-04-05 04:54:47 PDT
(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.
Comment 6 Build Bot 2019-04-05 05:13:21 PDT
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
Comment 7 Build Bot 2019-04-05 05:13:23 PDT
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
Comment 8 Build Bot 2019-04-05 05:27:08 PDT
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
Comment 9 Build Bot 2019-04-05 05:27:10 PDT
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
Comment 10 Build Bot 2019-04-05 05:55:51 PDT
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
Comment 11 Build Bot 2019-04-05 05:55:52 PDT
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
Comment 12 Build Bot 2019-04-05 06:03:48 PDT
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
Comment 13 Build Bot 2019-04-05 06:03:50 PDT
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
Comment 14 Build Bot 2019-04-05 10:07:35 PDT
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
Comment 15 Build Bot 2019-04-05 10:07:47 PDT
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
Comment 16 Manuel Rego Casasnovas 2019-04-10 05:56:15 PDT
Created attachment 367116 [details]
Patch
Comment 17 Build Bot 2019-04-10 07:01:00 PDT
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
Comment 18 Build Bot 2019-04-10 07:01:01 PDT
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
Comment 19 Build Bot 2019-04-10 07:15:17 PDT
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
Comment 20 Build Bot 2019-04-10 07:15:18 PDT
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
Comment 21 Build Bot 2019-04-10 07:46:24 PDT
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
Comment 22 Build Bot 2019-04-10 07:46:26 PDT
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
Comment 23 Build Bot 2019-04-10 07:52:10 PDT
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
Comment 24 Build Bot 2019-04-10 07:52:12 PDT
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
Comment 25 Build Bot 2019-04-10 08:31:59 PDT
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
Comment 26 Build Bot 2019-04-10 08:32:11 PDT
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
Comment 27 Manuel Rego Casasnovas 2019-04-12 02:09:25 PDT
Created attachment 367304 [details]
Patch
Comment 28 Manuel Rego Casasnovas 2019-04-12 03:30:14 PDT
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.
Comment 29 WebKit Commit Bot 2019-04-12 05:04:27 PDT
Comment on attachment 367304 [details]
Patch

Clearing flags on attachment: 367304

Committed r244213: <https://trac.webkit.org/changeset/244213>
Comment 30 WebKit Commit Bot 2019-04-12 05:04:29 PDT
All reviewed patches have been landed.  Closing bug.