Changeset 270073 in webkit


Ignore:
Timestamp:
Nov 20, 2020 1:33:25 AM (3 years ago)
Author:
svillar@igalia.com
Message:

[css-flex] Images as flex items should use the overridingLogicalHeight when defined to compute the logical width
​https://bugs.webkit.org/show_bug.cgi?id=218975

Reviewed by Manuel Rego Casasnovas.

LayoutTests/imported/w3c:

Replaced 19 FAIL by PASS expectations.

  • web-platform-tests/css/css-flexbox/image-as-flexitem-size-001v-expected.txt:
  • web-platform-tests/css/css-flexbox/image-as-flexitem-size-002-expected.txt:
  • web-platform-tests/css/css-flexbox/image-as-flexitem-size-005v-expected.txt:
  • web-platform-tests/css/css-flexbox/image-as-flexitem-size-006-expected.txt:
  • web-platform-tests/css/css-flexbox/image-as-flexitem-size-007v-expected.txt:

Source/WebCore:

RenderReplaced should use the overridingLogicalHeight whenever defined instead of the specified logical height to compute the logical
width using an intrinsic aspect ratio. The overriding height is set by flex containers that need to stretch/shrink their items.
The current code was not considering this case and thus, the intrinsic (non-stretched) logical height was used to compute the logical width,
meaning that the stretching set by the flexbox container was ignored.

Note that it isn't enough to check that there is an overriding height, we must also check that the replaced element has an intrinsic size.
Replaced elements with intrinsic ratios but without intrinsic sizes are handled in a separate code path (it's actually undefined behaviour).
That's why it isn't enough to verify that the element has an aspect ratio, because most SVG graphics actually have defined intrinsic ratios
but not intrinsic sizes.

This allows us to pass an additional 19 subtests in 5 flexbox WPT tests.

  • rendering/RenderReplaced.cpp:

(WebCore::RenderReplaced::computeReplacedLogicalWidth const): Use the overriding logical height if defined in presence of a valid aspect ratio.

Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r270048 r270073  
     12020-11-16  Sergio Villar Senin  <svillar@igalia.com>
     2
     3        [css-flex] Images as flex items should use the overridingLogicalHeight when defined to compute the logical width
     4        https://bugs.webkit.org/show_bug.cgi?id=218975
     5
     6        Reviewed by Manuel Rego Casasnovas.
     7
     8        Replaced 19 FAIL by PASS expectations.
     9
     10        * web-platform-tests/css/css-flexbox/image-as-flexitem-size-001v-expected.txt:
     11        * web-platform-tests/css/css-flexbox/image-as-flexitem-size-002-expected.txt:
     12        * web-platform-tests/css/css-flexbox/image-as-flexitem-size-005v-expected.txt:
     13        * web-platform-tests/css/css-flexbox/image-as-flexitem-size-006-expected.txt:
     14        * web-platform-tests/css/css-flexbox/image-as-flexitem-size-007v-expected.txt:
     15
    1162020-11-19  Chris Dumez  <cdumez@apple.com>
    217

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/image-as-flexitem-size-001v-expected.txt

    r269628 r270073  
    99PASS .flexbox > img 2
    1010PASS .flexbox > img 3
    11 FAIL .flexbox > img 4 assert_equals:
    12 <img src="support/solidblue.png" style="flex: 0 0 30px" data-expected-width="30" data-expected-height="30">
    13 height expected 30 but got 16
     11PASS .flexbox > img 4
    1412PASS .flexbox > img 5
    1513PASS .flexbox > img 6

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/image-as-flexitem-size-002-expected.txt

    r269628 r270073  
    99PASS .flexbox > img 2
    1010PASS .flexbox > img 3
    11 FAIL .flexbox > img 4 assert_equals:
    12 <img src="support/solidblue.png" style="flex: 0 0 30px" data-expected-width="30" data-expected-height="30">
    13 width expected 30 but got 16
     11PASS .flexbox > img 4
    1412PASS .flexbox > img 5
    1513PASS .flexbox > img 6

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/image-as-flexitem-size-005v-expected.txt

    r269628 r270073  
    66
    77
    8 FAIL .flexbox > img 1 assert_equals:
    9 <img src="support/solidblue.png" data-expected-width="40" data-expected-height="40">
    10 height expected 40 but got 16
    11 FAIL .flexbox > img 2 assert_equals:
    12 <img src="support/solidblue.png" style="width: 30px" data-expected-width="40" data-expected-height="40">
    13 height expected 40 but got 30
     8PASS .flexbox > img 1
     9PASS .flexbox > img 2
    1410PASS .flexbox > img 3
    15 FAIL .flexbox > img 4 assert_equals:
    16 <img src="support/solidblue.png" style="flex: 1 1 30px" data-expected-width="40" data-expected-height="40">
    17 height expected 40 but got 16
    18 FAIL .flexbox > img 5 assert_equals:
    19 <img src="support/solidblue.png" style="min-width: 34px" data-expected-width="40" data-expected-height="40">
    20 height expected 40 but got 34
    21 FAIL .flexbox > img 6 assert_equals:
    22 <img src="support/solidblue.png" style="min-height: 34px" data-expected-width="40" data-expected-height="40">
    23 height expected 40 but got 34
    24 FAIL .flexbox > img 7 assert_equals:
    25 <img src="support/solidblue.png" style="min-width: 30px;
    26                                               min-height: 34px" data-expected-width="40" data-expected-height="40">
    27 height expected 40 but got 34
    28 FAIL .flexbox > img 8 assert_equals:
    29 <img src="support/solidblue.png" style="min-width: 34px;
    30                                               min-height: 30px" data-expected-width="40" data-expected-height="40">
    31 height expected 40 but got 34
     11PASS .flexbox > img 4
     12PASS .flexbox > img 5
     13PASS .flexbox > img 6
     14PASS .flexbox > img 7
     15PASS .flexbox > img 8
    3216PASS .flexbox > img 9
    3317PASS .flexbox > img 10
    … …  
    3923PASS .flexbox > img 16
    4024PASS .flexbox > img 17
    41 FAIL .flexbox > img 18 assert_equals:
    42 <img src="support/solidblue.png" style="width: 10px;
    43                                               min-height: 30px" data-expected-width="40" data-expected-height="40">
    44 height expected 40 but got 30
     25PASS .flexbox > img 18
    4526

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/image-as-flexitem-size-006-expected.txt

    r269628 r270073  
    66
    77
    8 FAIL .flexbox > img 1 assert_equals:
    9 <img src="support/solidblue.png" data-expected-width="40" data-expected-height="40">
    10 width expected 40 but got 16
     8PASS .flexbox > img 1
    119PASS .flexbox > img 2
    12 FAIL .flexbox > img 3 assert_equals:
    13 <img src="support/solidblue.png" style="height: 30px" data-expected-width="40" data-expected-height="40">
    14 width expected 40 but got 30
    15 FAIL .flexbox > img 4 assert_equals:
    16 <img src="support/solidblue.png" style="flex: 1 1 30px" data-expected-width="40" data-expected-height="40">
    17 width expected 40 but got 16
    18 FAIL .flexbox > img 5 assert_equals:
    19 <img src="support/solidblue.png" style="min-width: 34px" data-expected-width="40" data-expected-height="40">
    20 width expected 40 but got 34
    21 FAIL .flexbox > img 6 assert_equals:
    22 <img src="support/solidblue.png" style="min-height: 34px" data-expected-width="40" data-expected-height="40">
    23 width expected 40 but got 34
    24 FAIL .flexbox > img 7 assert_equals:
    25 <img src="support/solidblue.png" style="min-width: 30px;
    26                                               min-height: 34px" data-expected-width="40" data-expected-height="40">
    27 width expected 40 but got 34
    28 FAIL .flexbox > img 8 assert_equals:
    29 <img src="support/solidblue.png" style="min-width: 34px;
    30                                               min-height: 30px" data-expected-width="40" data-expected-height="40">
    31 width expected 40 but got 34
     10PASS .flexbox > img 3
     11PASS .flexbox > img 4
     12PASS .flexbox > img 5
     13PASS .flexbox > img 6
     14PASS .flexbox > img 7
     15PASS .flexbox > img 8
    3216PASS .flexbox > img 9
    3317PASS .flexbox > img 10
    … …  
    3620PASS .flexbox > img 13
    3721PASS .flexbox > img 14
    38 FAIL .flexbox > img 15 assert_equals:
    39 <img src="support/solidblue.png" style="min-width: 30px;
    40                                               height: 10px" data-expected-width="40" data-expected-height="40">
    41 width expected 40 but got 30
     22PASS .flexbox > img 15
    4223PASS .flexbox > img 16
    4324PASS .flexbox > img 17

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/image-as-flexitem-size-007v-expected.txt

    r269628 r270073  
    1111<img src="support/solidblue.png" style="height: 30px" data-expected-width="32" data-expected-height="30">
    1212width expected 32 but got 30
    13 FAIL .flexbox > img 4 assert_equals:
    14 <img src="support/solidblue.png" style="flex: 0 0 30px" data-expected-width="30" data-expected-height="28">
    15 height expected 28 but got 20
     13PASS .flexbox > img 4
    1614PASS .flexbox > img 5
    1715PASS .flexbox > img 6

  • trunk/Source/WebCore/ChangeLog

    r270071 r270073  
     12020-11-16  Sergio Villar Senin  <svillar@igalia.com>
     2
     3        [css-flex] Images as flex items should use the overridingLogicalHeight when defined to compute the logical width
     4        https://bugs.webkit.org/show_bug.cgi?id=218975
     5
     6        Reviewed by Manuel Rego Casasnovas.
     7
     8        RenderReplaced should use the overridingLogicalHeight whenever defined instead of the specified logical height to compute the logical
     9        width using an intrinsic aspect ratio. The overriding height is set by flex containers that need to stretch/shrink their items.
     10        The current code was not considering this case and thus, the intrinsic (non-stretched) logical height was used to compute the logical width,
     11        meaning that the stretching set by the flexbox container was ignored.
     12
     13        Note that it isn't enough to check that there is an overriding height, we must also check that the replaced element has an intrinsic size.
     14        Replaced elements with intrinsic ratios but without intrinsic sizes are handled in a separate code path (it's actually undefined behaviour).
     15        That's why it isn't enough to verify that the element has an aspect ratio, because most SVG graphics actually have defined intrinsic ratios
     16        but not intrinsic sizes.
     17
     18        This allows us to pass an additional 19 subtests in 5 flexbox WPT tests.
     19
     20        * rendering/RenderReplaced.cpp:
     21        (WebCore::RenderReplaced::computeReplacedLogicalWidth const): Use the overriding logical height if defined in presence of a valid aspect ratio.
     22
    1232020-11-19  Fujii Hironori  <Hironori.Fujii@sony.com>
    224

  • trunk/Source/WebCore/rendering/RenderReplaced.cpp

    r269820 r270073  
    518518        bool computedHeightIsAuto = style().logicalHeight().isAuto();
    519519        bool hasIntrinsicWidth = constrainedSize.width() > 0;
     520        bool hasIntrinsicHeight = constrainedSize.height() > 0;
     521
     522        // For flex items where the logical height has been overriden then we should use that size to compute the replaced width as long as the flex item has
     523        // an intrinsic size. It is possible (indeed, common) for an SVG graphic to have an intrinsic aspect ratio but not to have an intrinsic width or height.
     524        // There are also elements with intrinsic sizes but without intrinsic ratio (like an iframe). We cannot apply this rule to grid items because the grid
     525        // container can distort aspect ratios in case of "align-self: stretch" for example (see https://drafts.csswg.org/css-grid/#grid-item-sizing).
     526        if (intrinsicRatio && isFlexItem() && hasOverridingLogicalHeight() && hasIntrinsicWidth && hasIntrinsicHeight)
     527            return computeReplacedLogicalWidthRespectingMinMaxWidth(roundToInt(round(overridingContentLogicalHeight() * intrinsicRatio)), shouldComputePreferred);
    520528
    521529        // If 'height' and 'width' both have computed values of 'auto' and the element also has an intrinsic width, then that intrinsic width is the used value of 'width'.
    … …  
    523531            return computeReplacedLogicalWidthRespectingMinMaxWidth(constrainedSize.width(), shouldComputePreferred);
    524532
    525         bool hasIntrinsicHeight = constrainedSize.height() > 0;
    526533        if (intrinsicRatio) {
    527534            // If 'height' and 'width' both have computed values of 'auto' and the element has no intrinsic width, but does have an intrinsic height and intrinsic ratio;
Note: See TracChangeset for help on using the changeset viewer.