Bug 216145 - [css-grid] Incorrectly stretched SVGs without an aspect-ratio
Summary: [css-grid] Incorrectly stretched SVGs without an aspect-ratio
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
: 236792 (view as bug list)
Depends on: 223504
Blocks:
  Show dependency treegraph
 
Reported: 2020-09-03 14:56 PDT by Oriol Brufau
Modified: 2023-11-20 16:19 PST (History)
19 users (show)

See Also:


Attachments
Patch (6.71 KB, text/plain)
2021-03-18 13:44 PDT, zsun
ews-feeder: commit-queue-
Details
expectations (2.03 KB, patch)
2022-02-17 11:35 PST, Dawn Morningstar
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oriol Brufau 2020-09-03 14:56:43 PDT
The following WPT tests are failing in WebKit:

css/css-grid/alignment/grid-item-no-aspect-ratio-stretch-5.html
css/css-grid/alignment/grid-item-no-aspect-ratio-stretch-6.html
css/css-grid/alignment/grid-item-no-aspect-ratio-stretch-7.html
css/css-grid/alignment/grid-item-no-aspect-ratio-stretch-8.html
css/css-grid/alignment/grid-item-no-aspect-ratio-stretch-9.html
css/css-grid/alignment/grid-item-no-aspect-ratio-stretch-10.html

https://wpt.fyi/results/css/css-grid/alignment?label=master&label=experimental&aligned&q=grid-item-no-aspect-ratio-stretch

Most of them (except 6 and 7) also fail in Chromium: https://crbug.com/1114013
Comment 1 Radar WebKit Bug Importer 2020-09-10 14:57:16 PDT
<rdar://problem/68666896>
Comment 2 zsun 2021-03-18 13:44:55 PDT
Created attachment 423644 [details]
Patch
Comment 3 Javier Fernandez 2021-03-18 14:29:03 PDT
Comment on attachment 423644 [details]
Patch

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

The patch looks good, but it doesn't fix all the tests that has been identified as failing due to this bug. We should either fix all of them or create a different bug, which this will depend on, and provide this patch as fix for that.

> Source/WebCore/ChangeLog:8
> +        When SVG element has fixed width or height as 0, it should also be reflected as true for has

s/for has/for

> Source/WebCore/ChangeLog:10
> +        RenderReplaced::computeReplacedLogicalHeight. This change is to differentiate the case when

Could you explain a bit more the case we want to differentiate from ?

> Source/WebCore/platform/graphics/FloatSize.h:75
> +    bool hasWidth = false;

Give than we set it to true if the Length value assigned to FloatSize attributes is Fixed or not, wouldn't be better to define these attributes as isFixed ?

> Source/WebCore/rendering/svg/RenderSVGRoot.cpp:83
> +    intrinsicSize.hasWidth = svgSVGElement().intrinsicWidth().isFixed();

The function computeIntrinsicRatioInformation is overridden by RenderImage as well; shouldn't this class set values to the hasWidth and hasHeight as well ? Otherwise, we'll use the default values for these FloatSize's fields in the RenderReplaced logic.

Additionally, are you sure that isFixed is enough ? Wouldn't be better to check use Length::isSpeficied() instead ? (it includes percent or calculated as well).
Comment 4 zsun 2021-03-19 03:29:19 PDT
Fixes on 

css/css-grid/alignment/grid-item-no-aspect-ratio-stretch-6.html
css/css-grid/alignment/grid-item-no-aspect-ratio-stretch-7.html

are now at 

https://bugs.webkit.org/show_bug.cgi?id=223504
Comment 5 Dawn Morningstar 2022-02-17 11:35:41 PST
Created attachment 452396 [details]
expectations
Comment 6 Dawn Morningstar 2022-02-17 11:36:27 PST
*** Bug 236792 has been marked as a duplicate of this bug. ***
Comment 7 Robert Jenner 2022-02-17 11:39:09 PST
Comment on attachment 452396 [details]
expectations

Clearing flags on attachment: 452396

Committed r290038 (247416@trunk): <https://commits.webkit.org/247416@trunk>
Comment 8 Dawn Morningstar 2022-02-17 11:50:54 PST
(In reply to Robert Jenner from comment #7)
> Comment on attachment 452396 [details]
> expectations
> 
> Clearing flags on attachment: 452396
> 
> Committed r290038 (247416@trunk): <https://commits.webkit.org/247416@trunk>

I authored this commit, and Robert landed it for me, the purpose of the commit was to change the expectations on the two tests that did not receive changes, to relieve the EWS queue backlog.
Comment 9 zsun 2022-05-13 02:26:48 PDT
My intention was only to fix 

css/css-grid/alignment/grid-item-no-aspect-ratio-stretch-6.html
css/css-grid/alignment/grid-item-no-aspect-ratio-stretch-7.html

and they are being addressed at bug 223504.