| Summary: | [css-flexbox] Only update the intrinsic height if we don't have override height | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Sergio Villar Senin <svillar> | ||||
| Component: | New Bugs | Assignee: | Sergio Villar Senin <svillar> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | changseok, clopez, darin, esprehn+autocc, ews-watchlist, glenn, graouts, jfernandez, kondapallykalyan, pdr, svillar, webkit-bug-importer, zalan | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Sergio Villar Senin
2020-08-11 04:43:17 PDT
Created attachment 406375 [details]
Patch
Comment on attachment 406375 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406375&action=review > Source/WebCore/rendering/RenderBox.cpp:2781 > + if (isFloatingOrOutOfFlowPositioned() || !parent() || !parent()->isFlexibleBox() || hasOverrideContentLogicalHeight()) I think Grid items with stretch alignment will have the same problem with this cache. Wouldn't make more sense to just check for the HasOverrideContentLogicalHeight and remove the clauses about flexible box ? *** Bug 210088 has been marked as a duplicate of this bug. *** (In reply to Javier Fernandez from comment #2) > Comment on attachment 406375 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=406375&action=review > > > Source/WebCore/rendering/RenderBox.cpp:2781 > > + if (isFloatingOrOutOfFlowPositioned() || !parent() || !parent()->isFlexibleBox() || hasOverrideContentLogicalHeight()) > > I think Grid items with stretch alignment will have the same problem with > this cache. Wouldn't make more sense to just check for the > HasOverrideContentLogicalHeight and remove the clauses about flexible box ? Right, we might eventually need it for grid too. However we cannot remove the checks now beause that triggers an infinite recursion between layoutBlock() <-> relayoutToAvoidWidows() that should be fixed as part of another patch. I could include a FIXME if you want. Comment on attachment 406375 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406375&action=review >>> Source/WebCore/rendering/RenderBox.cpp:2781 >>> + if (isFloatingOrOutOfFlowPositioned() || !parent() || !parent()->isFlexibleBox() || hasOverrideContentLogicalHeight()) >> >> I think Grid items with stretch alignment will have the same problem with this cache. Wouldn't make more sense to just check for the HasOverrideContentLogicalHeight and remove the clauses about flexible box ? > > Right, we might eventually need it for grid too. However we cannot remove the checks now beause that triggers an infinite recursion between > > layoutBlock() <-> relayoutToAvoidWidows() > > that should be fixed as part of another patch. I could include a FIXME if you want. Yes, please, add that FIXME. Comment on attachment 406375 [details]
Patch
r=me
Committed r265497: <https://trac.webkit.org/changeset/265497> |