Bug 228105 – [css-grid] svg image as grid items should use the overriding logical width/height when defined to compute the logical height/width

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

svg image as grid items should use the overriding logical width/height when defined to compute the logical height/width as well.

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

Created attachment 433869 [details]
Patch

[Javier Fernandez]

Comment on attachment 433869 [details]
Patch

r=me

[EWS]

Committed r280290 (239948@main): <https://commits.webkit.org/239948@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433869 [details].

[Radar WebKit Bug Importer]

<rdar://problem/81093461>

[Rob Buis]

I think a lambda or in this case a free function would be preferred here to keep the if statement readable.

[Javier Fernandez]

(In reply to Rob Buis from comment #6)
> I think a lambda or in this case a free function would be preferred here to
> keep the if statement readable.

yeah, I agree. Ziran, could you consider Rob's idea in a follow up patch ?

[WebKit Commit Bot]

Re-opened since this is blocked by bug 233184

[Javier Fernandez]

Comment on attachment 433869 [details]
Patch

What about the Rob's suggestion in #c6 ?

(In reply to Javier Fernandez from comment #9)
> Comment on attachment 433869 [details]
> Patch
> 
> What about the Rob's suggestion in #c6 ?

I have a patch to upload that addresses Rob's suggestion in #c6. We probably need to land bug 228022 first to avoid rebasing.

Created attachment 444665 [details]
Patch

[Javier Fernandez]

Comment on attachment 444665 [details]
Patch

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

r=me

> Source/WebCore/rendering/RenderReplaced.cpp:534
> +static inline bool hasIntrinsicSize(RenderBox*contentRenderer, bool hasIntrinsicWidth, bool hasIntrinsicHeight )

nit: s/*contentRender/* contentRender

> Source/WebCore/rendering/RenderReplaced.cpp:538
> +    if (contentRenderer && contentRenderer->isSVGRoot() && (hasIntrinsicWidth || hasIntrinsicHeight))

Just a suggestion; what about doing this like:

if (hasIntrinsicWidth || hasIntrinsicHeight))
  return contentRenderer && contentRenderer->isSVGRoot()

The idea is to make clear that we only consider single-axis size as intrinsic in the case of SVG root. For any other case we would expect both, width and height to be intrinsic.

Created attachment 444790 [details]
Patch

[EWS]

Committed r286100 (244487@main): <https://commits.webkit.org/244487@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 444790 [details].