Bug 228105 - [css-grid] svg image as grid items should use the overriding logical width/height when defined to compute the logical height/width
Summary: [css-grid] svg image as grid items should use the overriding logical width/he...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zsun
URL:
Keywords: InRadar
: 227900 (view as bug list)
Depends on: 233184
Blocks:
  Show dependency tree / graph
 
Reported: 2021-07-20 05:04 PDT by zsun
Modified: 2021-11-21 10:22 PST (History)
12 users (show)

See Also:

Attachments
Patch (6.46 KB, patch)
2021-07-20 05:22 PDT, zsun 		no flags Details | Formatted Diff | Diff
Patch (6.91 KB, patch)
2021-11-18 06:13 PST, zsun 		no flags Details | Formatted Diff | Diff
Patch (6.90 KB, patch)
2021-11-19 01:33 PST, zsun 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description zsun 2021-07-20 05:04:30 PDT 
svg image as grid items should use the overriding logical width/height when defined to compute the logical height/width as well.
Comment 1 zsun 2021-07-20 05:20:08 PDT 
*** Bug 227900 has been marked as a duplicate of this bug. ***
Comment 2 zsun 2021-07-20 05:22:27 PDT 
Created attachment 433869 [details]
Patch
Comment 3 Javier Fernandez 2021-07-22 16:15:28 PDT 
Comment on attachment 433869 [details]
Patch

r=me
Comment 4 EWS 2021-07-26 00:42:52 PDT 
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].
Comment 5 Radar WebKit Bug Importer 2021-07-26 00:43:17 PDT 
<rdar://problem/81093461>
Comment 6 Rob Buis 2021-07-26 02:30:54 PDT 
I think a lambda or in this case a free function would be preferred here to keep the if statement readable.
Comment 7 Javier Fernandez 2021-07-26 03:04:16 PDT 
(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 ?
Comment 8 WebKit Commit Bot 2021-11-16 03:38:01 PST 
Re-opened since this is blocked by bug 233184
Comment 9 Javier Fernandez 2021-11-17 15:10:17 PST 
Comment on attachment 433869 [details]
Patch

What about the Rob's suggestion in #c6 ?
Comment 10 zsun 2021-11-18 01:00:16 PST 
(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.
Comment 11 zsun 2021-11-18 06:13:52 PST 
Created attachment 444665 [details]
Patch
Comment 12 Javier Fernandez 2021-11-19 01:06:09 PST 
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.
Comment 13 zsun 2021-11-19 01:33:12 PST 
Created attachment 444790 [details]
Patch
Comment 14 EWS 2021-11-21 10:22:09 PST 
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].