| Summary: | [css-flexbox] REGRESSION(r266695): content inside a `<button>` inside a flex container has a height of `0` without a declared `min-height` | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Devin Rousso <hi> | ||||||||||
| Component: | Layout and Rendering | Assignee: | Sergio Villar Senin <svillar> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | bdakin, bfulgham, changseok, esprehn+autocc, ews-watchlist, glenn, hi, kondapallykalyan, pdr, simon.fraser, svillar, webkit-bug-importer, zalan | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Bug Depends on: | 210089 | ||||||||||||
| Bug Blocks: | |||||||||||||
| Attachments: |
|
||||||||||||
Checking Created attachment 418383 [details] Rendering in different engines JFTR I'm attaching the rendering in 3 different engines, WebKit (pre-r266695), Chromium and Firefox. As you can see it's different in all of them, so it's unclear what's the right one. Anyway I'll get back the old behaviour for buttons. (In reply to Sergio Villar Senin from comment #3) > Created attachment 418383 [details] > Rendering in different engines > > JFTR I'm attaching the rendering in 3 different engines, WebKit > (pre-r266695), Chromium and Firefox. As you can see it's different in all of > them, so it's unclear what's the right one. Ah the example contains a button, which has different UA styles in every single engine. Nevermind. (In reply to Sergio Villar Senin from comment #4) > (In reply to Sergio Villar Senin from comment #3) > > Created attachment 418383 [details] > > Rendering in different engines > > > > JFTR I'm attaching the rendering in 3 different engines, WebKit (pre-r266695), Chromium and Firefox. As you can see it's different in all of them, so it's unclear what's the right one. > > Ah the example contains a button, which has different UA styles in every single engine. Nevermind. Sorry I should've clarified. The issue is that the `background-color: red;` isn't shown after r266695 (unless you add a `min-height: 0;` to the parent of `<button>`). Created attachment 418631 [details]
Patch
Zalan mind taking a look? You did the optimization so you'd likely be the best reviewer for this. Created attachment 418634 [details]
Patch
Comment on attachment 418634 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418634&action=review > Source/WebCore/ChangeLog:14 > + FlexibleBoxImpl's like RenderButton wrap their DOM children in anonymous blocks. Those anonymous blocks are Is it really always the case? -and what exactly do you mean by DOM children. This is the render tree, we don't have DOM objects here. > Source/WebCore/rendering/RenderFlexibleBox.cpp:1639 > + auto* descendants = percentHeightDescendants(); I'd prefer writing it out like this auto& descendants = *percentHeightDescendants(); so that if "descendants" is later used in this function no one starts null checking it. Comment on attachment 418634 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418634&action=review >> Source/WebCore/ChangeLog:14 >> + FlexibleBoxImpl's like RenderButton wrap their DOM children in anonymous blocks. Those anonymous blocks are > > Is it really always the case? -and what exactly do you mean by DOM children. This is the render tree, we don't have DOM objects here. It's badly explained indeed. I wanted to say that <button> is a FlexibleBoxImpl which wraps the <button> children in anonymous blocks. I should not have mixed the render objects with the dom ones in the same explanation. >> Source/WebCore/rendering/RenderFlexibleBox.cpp:1639 >> + auto* descendants = percentHeightDescendants(); > > I'd prefer writing it out like this > auto& descendants = *percentHeightDescendants(); > so that if "descendants" is later used in this function no one starts null checking it. OK. Committed r272054: <https://trac.webkit.org/changeset/272054> |
Created attachment 418333 [details] [HTML] reduction Inspecting the attached `reduction.html`, the Computed details sidebar panel shows a height of `0` for the child of the `<button>`. After adding `min-height: 0;` to the parent of the `<button>` (which is the child of the `display: flex;`), however, the height then shows `50%` (`100px`) and the `background-color: red;` becomes visible.