⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 285998 in webkit


Ignore:
Timestamp:
Nov 18, 2021, 5:12:28 AM (5 years ago)
Author:
Ziran Sun
Message:

Images as grid items should use the overridingLogicalWidth when defined to compute the logical Height
​https://bugs.webkit.org/show_bug.cgi?id=228022

LayoutTests/imported/w3c:

Reviewed by Javier Fernandez.

  • web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-029-expected.txt:
  • web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-030-expected.txt:
  • web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-031-expected.txt:
  • web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-035-expected.txt:
  • web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-036-expected.txt:

Source/WebCore:

This is a re-land of r280078 and r282008. r280078 has caused a crash. Details on the crash test case is
at ​https://bugs.webkit.org/show_bug.cgi?id=232922. The reason for the crash is that, when recomputing the
intrinsic width we need to recompute the intrinsic width of the caller, which is the grid item, rather than
the grid in this case.

This patch addresses the original issue and the cause for the crash.

When computing the logical height using an intrinsic aspect ratio, RenderReplaced should use the
overridingLogicalWidth whenever defined just as how it does for flex items. This change is to
replace the use of intrinsic (non-stretched) logical width in current code with the overridingLogicalWidth.

The current code doesn't set overringLogicalWidth while the width changes while stretches along Row Axis.
This change adds the logic in.

Reviewed by Javier Fernandez.

  • rendering/RenderGrid.cpp:

(WebCore::RenderGrid::applyStretchAlignmentToChildIfNeeded):

  • rendering/RenderReplaced.cpp:

(WebCore::RenderReplaced::computeReplacedLogicalHeight const):

LayoutTests:

Reviewed by Javier Fernandez.

Location:
trunk
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r285993 r285998  
     12021-11-18  Ziran Sun  <zsun@igalia.com>
     2
     3        Images as grid items should use the overridingLogicalWidth when defined to compute the logical Height
     4        https://bugs.webkit.org/show_bug.cgi?id=228022
     5
     6        Reviewed by Javier Fernandez.
     7
     8        * TestExpectations: unskip tests that are now passing.
     9
    1102021-11-18  Youenn Fablet  <youenn@apple.com>
    211

  • trunk/LayoutTests/TestExpectations

    r285989 r285998  
    13891389webkit.org/b/216146 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-baseline-align-001.html [ ImageOnlyFailure ]
    13901390webkit.org/b/216146 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-baseline-justify-001.html [ ImageOnlyFailure ]
    1391 imported/w3c/web-platform-tests/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-002.html [ ImageOnlyFailure ]
    1392 imported/w3c/web-platform-tests/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-007.html [ ImageOnlyFailure ]
    1393 imported/w3c/web-platform-tests/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-008.html [ ImageOnlyFailure ]
    13941391
    13951392imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-dynamic-001.html [ ImageOnlyFailure ]
    … …  
    43274324webkit.org/b/227900 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-item-aspect-ratio-stretch-1.html [ ImageOnlyFailure ]
    43284325webkit.org/b/227900 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-item-aspect-ratio-stretch-2.html [ ImageOnlyFailure ]
    4329 webkit.org/b/227900 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-item-aspect-ratio-stretch-3.html [ ImageOnlyFailure ]
    4330 webkit.org/b/227900 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-item-aspect-ratio-stretch-4.html [ ImageOnlyFailure ]
    43314326webkit.org/b/212246 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-baseline-align-cycles-001.html [ ImageOnlyFailure ]
    43324327webkit.org/b/231021 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-inline-baseline.html [ ImageOnlyFailure ]

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r285995 r285998  
     12021-11-18  Ziran Sun  <zsun@igalia.com>
     2
     3        Images as grid items should use the overridingLogicalWidth when defined to compute the logical Height
     4        https://bugs.webkit.org/show_bug.cgi?id=228022
     5
     6        Reviewed by Javier Fernandez.
     7
     8        * web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-029-expected.txt:
     9        * web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-030-expected.txt:
     10        * web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-031-expected.txt:
     11        * web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-035-expected.txt:
     12        * web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-036-expected.txt:
     13
    1142021-11-18  Arcady Goldmints-Orlov  <agoldmints@igalia.com>
    215

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-029-expected.txt

    r285857 r285998  
    11
    22
    3 FAIL .before 1 assert_equals:
    4 <img id="item" src="support/100x100-green.png" class=" before" data-expected-width="200" data-expected-height="200">
    5 height expected 200 but got 100
     3PASS .before 1
    64PASS .after 2
    75

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-030-expected.txt

    r285857 r285998  
    22
    33PASS .before 1
    4 FAIL .after 2 assert_equals:
    5 <img id="item" src="support/100x100-green.png" class=" before after" data-expected-width="200" data-expected-height="200">
    6 height expected 200 but got 100
     4PASS .after 2
    75

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-031-expected.txt

    r285857 r285998  
    11
    22
    3 FAIL .before 1 assert_equals:
    4 <img id="item" src="support/100x100-green.png" class=" before" data-expected-width="200" data-expected-height="200">
    5 height expected 200 but got 100
     3PASS .before 1
    64PASS .after 2
    75

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-035-expected.txt

    r285857 r285998  
    22
    33PASS .before 1
    4 FAIL .after 2 assert_equals:
    5 <img id="item" src="support/100x100-green.png" class=" before after" data-expected-width="80" data-expected-height="80">
    6 height expected 80 but got 100
     4PASS .after 2
    75

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-alignment-implies-size-change-036-expected.txt

    r285857 r285998  
    11
    22
    3 FAIL .before 1 assert_equals:
    4 <img id="item" src="support/100x100-green.png" class=" before" data-expected-width="80" data-expected-height="80">
    5 height expected 80 but got 100
     3PASS .before 1
    64PASS .after 2
    75

  • trunk/LayoutTests/platform/win/TestExpectations

    r285936 r285998  
    38253825webkit.org/b/177216 fast/images/animated-image-mp4.html [ Skip ]
    38263826
    3827 webkit.org/b/228325 fast/images/exif-orientation-composited.html [ Pass ImageOnlyFailure ]
    3828 
    38293827webkit.org/b/177234 http/wpt/resource-timing/rt-cors.html [ Skip ]
    38303828webkit.org/b/177234 http/wpt/resource-timing/rt-cors.worker.html [ Skip ]

  • trunk/Source/WebCore/ChangeLog

    r285997 r285998  
     12021-11-18  Ziran Sun  <zsun@igalia.com>
     2
     3        Images as grid items should use the overridingLogicalWidth when defined to compute the logical Height
     4        https://bugs.webkit.org/show_bug.cgi?id=228022
     5
     6        This is a re-land of r280078 and r282008. r280078 has caused a crash. Details on the crash test case is
     7        at https://bugs.webkit.org/show_bug.cgi?id=232922. The reason for the crash is that, when recomputing the
     8        intrinsic width we need to recompute the intrinsic width of the caller, which is the grid item, rather than
     9        the grid in this case.
     10       
     11        This patch addresses the original issue and the cause for the crash.
     12
     13        When computing the logical height using an intrinsic aspect ratio, RenderReplaced should use the
     14        overridingLogicalWidth whenever defined just as how it does for flex items. This change is to
     15        replace the use of intrinsic (non-stretched) logical width in current code with the overridingLogicalWidth.
     16
     17        The current code doesn't set overringLogicalWidth while the width changes while stretches along Row Axis.
     18        This change adds the logic in.
     19
     20        Reviewed by Javier Fernandez.
     21
     22        * rendering/RenderGrid.cpp:
     23        (WebCore::RenderGrid::applyStretchAlignmentToChildIfNeeded):
     24        * rendering/RenderReplaced.cpp:
     25        (WebCore::RenderReplaced::computeReplacedLogicalHeight const):
     26
    1272021-10-26  Carlos Garcia Campos  <cgarcia@igalia.com>
    228

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

    r285987 r285998  
    11841184{
    11851185    ASSERT(child.overridingContainingBlockContentLogicalHeight());
    1186 
    1187     // We clear height override values because we will decide now whether it's allowed or
     1186    ASSERT(child.overridingContainingBlockContentLogicalWidth());
     1187
     1188    // We clear height and width override values because we will decide now whether it's allowed or
    11881189    // not, evaluating the conditions which might have changed since the old values were set.
    11891190    child.clearOverridingLogicalHeight();
     1191    child.clearOverridingLogicalWidth();
    11901192
    11911193    GridTrackSizingDirection childBlockDirection = GridLayoutFunctions::flowAwareDirectionForChild(*this, child, ForRows);
     1194    GridTrackSizingDirection childInlineDirection = GridLayoutFunctions::flowAwareDirectionForChild(*this, child, ForColumns);
    11921195    bool blockFlowIsColumnAxis = childBlockDirection == ForRows;
    11931196    bool allowedToStretchChildBlockSize = blockFlowIsColumnAxis ? allowedToStretchChildAlongColumnAxis(child) : allowedToStretchChildAlongRowAxis(child);
    … …  
    12051208            child.setNeedsLayout(MarkOnlyThis);
    12061209        }
    1207     }
     1210    } else if (!allowedToStretchChildBlockSize && allowedToStretchChildAlongRowAxis(child)) {
     1211        LayoutUnit stretchedLogicalWidth = availableAlignmentSpaceForChildBeforeStretching(GridLayoutFunctions::overridingContainingBlockContentSizeForChild(child, childInlineDirection).value(), child);
     1212        LayoutUnit desiredLogicalWidth = child.constrainLogicalWidthInFragmentByMinMax(stretchedLogicalWidth, contentWidth(), *this, nullptr);
     1213        child.setOverridingLogicalWidth(desiredLogicalWidth);
     1214        if (desiredLogicalWidth != child.logicalWidth())
     1215            child.setNeedsLayout(MarkOnlyThis);
     1216    }
    12081217}
    12091218

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

    r285987 r285998  
    624624
    625625    // See computeReplacedLogicalHeight() for a similar check for heights.
    626     if (intrinsicRatio && isFlexItem() && hasOverridingLogicalWidth() && hasIntrinsicHeight && hasIntrinsicWidth)
     626    if (intrinsicRatio && (isFlexItem() || isGridItem()) && hasOverridingLogicalWidth() && hasIntrinsicHeight && hasIntrinsicWidth)
    627627        return computeReplacedLogicalHeightRespectingMinMaxHeight(roundToInt(round(overridingContentLogicalWidth() / intrinsicRatio)));
    628628
Note: See TracChangeset for help on using the changeset viewer.