[css-flexbox] Only update the intrinsic height if we don't have override height

[Sergio Villar Senin]

[css-flexbox] Don't update the intrinsic height if we don't have override height

[Sergio Villar Senin]

Created attachment 406375 [details]
Patch

[Javier Fernandez]

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 ?

[Sergio Villar Senin]

*** Bug 210088 has been marked as a duplicate of this bug. ***

[Sergio Villar Senin]

(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.

[Javier Fernandez]

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.

[Javier Fernandez]

Comment on attachment 406375 [details]
Patch

r=me

[Sergio Villar Senin]

Committed r265497: <https://trac.webkit.org/changeset/265497>

[Radar WebKit Bug Importer]

<rdar://problem/66840803>