Changeset 285923 in webkit

Timestamp:

Nov 17, 2021, 4:31:09 AM (5 years ago)

Author:

commit-queue@webkit.org

Message:

Limit logical width over-constrained direction check
​https://bugs.webkit.org/show_bug.cgi?id=233189

Patch by Rob Buis <​rbuis@igalia.com> on 2021-11-17
Reviewed by Sergio Villar Senin.

Source/WebCore:

In computePositionedLogicalWidthUsing the over-constrained case always checks for RTL
direction, but this should not be done if we are not dealing with left and right properties,
i.e. only when we are not orthogonal with the container.
Finally computeLogicalLeftPositionedOffset should not do the flipping logic in the over-constrained case.

• rendering/RenderBox.cpp:

(WebCore::computeLogicalLeftPositionedOffset):
(WebCore::RenderBox::computePositionedLogicalWidthUsing const):
(WebCore::RenderBox::computePositionedLogicalWidthReplaced const):

LayoutTests:

• TestExpectations: Removed 4 tests that are now passing.
• platform/ios/TestExpectations: Skipped several tests that show 1px diff in the result.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• LayoutTests/platform/ios/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBox.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-11-17  Rob Buis  <rbuis@igalia.com>
+
+        Limit logical width over-constrained direction check
+        https://bugs.webkit.org/show_bug.cgi?id=233189
+
+        Reviewed by Sergio Villar Senin.
+
+        * TestExpectations: Removed 4 tests that are now passing.
+        * platform/ios/TestExpectations: Skipped several tests that show 1px diff in the result.
+
 2021-11-17  Antti Koivisto  <antti@apple.com>
 

trunk/LayoutTests/TestExpectations

@@ -4035 +4035 @@
 webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/abs-pos-non-replaced-icb-vrl-030.xht [ ImageOnlyFailure ]
 webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/abs-pos-non-replaced-vlr-095.xht [ ImageOnlyFailure ]
-webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/abs-pos-non-replaced-vlr-225.xht [ ImageOnlyFailure ]
-webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/abs-pos-non-replaced-vlr-229.xht [ ImageOnlyFailure ]
 webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/abs-pos-non-replaced-vrl-088.xht [ ImageOnlyFailure ]
-webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/abs-pos-non-replaced-vrl-220.xht [ ImageOnlyFailure ]
-webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/abs-pos-non-replaced-vrl-224.xht [ ImageOnlyFailure ]
 webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/available-size-001.html [ ImageOnlyFailure ]
 webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/available-size-002.html [ ImageOnlyFailure ]

trunk/LayoutTests/platform/ios/TestExpectations

@@ -3533 +3533 @@
 webkit.org/b/229397 imported/w3c/web-platform-tests/css/css-writing-modes/abs-pos-non-replaced-vrl-204.xht [ ImageOnlyFailure ]
 webkit.org/b/229397 imported/w3c/web-platform-tests/css/css-writing-modes/abs-pos-non-replaced-vrl-208.xht [ ImageOnlyFailure ]
+webkit.org/b/229397 imported/w3c/web-platform-tests/css/css-writing-modes/abs-pos-non-replaced-vrl-220.xht [ ImageOnlyFailure ]
+webkit.org/b/229397 imported/w3c/web-platform-tests/css/css-writing-modes/abs-pos-non-replaced-vrl-224.xht [ ImageOnlyFailure ]
+webkit.org/b/229397 imported/w3c/web-platform-tests/css/css-writing-modes/abs-pos-non-replaced-vlr-225.xht [ ImageOnlyFailure ]
+webkit.org/b/229397 imported/w3c/web-platform-tests/css/css-writing-modes/abs-pos-non-replaced-vlr-229.xht [ ImageOnlyFailure ]
 webkit.org/b/229397 imported/w3c/web-platform-tests/css/css-writing-modes/clip-rect-vrl-002.xht [ ImageOnlyFailure ]
 webkit.org/b/229397 imported/w3c/web-platform-tests/css/css-writing-modes/clip-rect-vrl-004.xht [ ImageOnlyFailure ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-11-17  Rob Buis  <rbuis@igalia.com>
+
+        Limit logical width over-constrained direction check
+        https://bugs.webkit.org/show_bug.cgi?id=233189
+
+        Reviewed by Sergio Villar Senin.
+
+        In computePositionedLogicalWidthUsing the over-constrained case always checks for RTL
+        direction, but this should not be done if we are not dealing with left and right properties,
+        i.e. only when we are not orthogonal with the container.
+        Finally computeLogicalLeftPositionedOffset should not do the flipping logic in the over-constrained case.
+
+        * rendering/RenderBox.cpp:
+        (WebCore::computeLogicalLeftPositionedOffset):
+        (WebCore::RenderBox::computePositionedLogicalWidthUsing const):
+        (WebCore::RenderBox::computePositionedLogicalWidthReplaced const):
+
 2021-11-17  Antoine Quint  <graouts@webkit.org>
 

trunk/Source/WebCore/rendering/RenderBox.cpp

@@ -3990 +3990 @@
 }
 
-static void computeLogicalLeftPositionedOffset(LayoutUnit& logicalLeftPos, const RenderBox* child, LayoutUnit logicalWidthValue, const RenderBoxModelObject& containerBlock, LayoutUnit containerLogicalWidth)
-{
-    auto logicalLeftAndRightAreAuto = child->style().logicalLeft().isAuto() && child->style().logicalRight().isAuto();
+static void computeLogicalLeftPositionedOffset(LayoutUnit& logicalLeftPos, const RenderBox* child, LayoutUnit logicalWidthValue, const RenderBoxModelObject& containerBlock, LayoutUnit containerLogicalWidth, bool logicalLeftIsAuto, bool logicalRightIsAuto)
+{
+    auto logicalLeftAndRightAreAuto = logicalLeftIsAuto && logicalRightIsAuto;
+    bool isOverconstrained = !logicalLeftIsAuto && !logicalRightIsAuto && !child->style().logicalWidth().isAuto();
     // Deal with differing writing modes here. Our offset needs to be in the containing block's coordinate space. If the containing block is flipped
     // along this axis, then we need to flip the coordinate. Auto positioned items do not need this correction as it was properly handled in
     // computeInlineStaticDistance().
-    if (isOrthogonal(*child, containerBlock) && !logicalLeftAndRightAreAuto && containerBlock.style().isFlippedBlocksWritingMode()) {
+    if (isOrthogonal(*child, containerBlock) && !logicalLeftAndRightAreAuto && !isOverconstrained && containerBlock.style().isFlippedBlocksWritingMode()) {
         logicalLeftPos = containerLogicalWidth - logicalWidthValue - logicalLeftPos;
         logicalLeftPos += (child->isHorizontalWritingMode() ? containerBlock.borderRight() : containerBlock.borderBottom());
@@ -4090 +4091 @@
             // Use the containing block's direction rather than the parent block's
             // per CSS 2.1 reference test abspos-non-replaced-width-margin-000.
-            if (containerDirection == TextDirection::RTL)
+            if (!isOrthogonal(*this, containerBlock) && containerDirection == TextDirection::RTL)
                 logicalLeftValue = (availableSpace + logicalLeftValue) - marginLogicalLeftValue - marginLogicalRightValue;
         }
@@ -4194 +4195 @@
 
     computedValues.m_position = logicalLeftValue + marginLogicalLeftValue;
-    computeLogicalLeftPositionedOffset(computedValues.m_position, this, computedValues.m_extent + bordersPlusPadding, containerBlock, containerLogicalWidth);
+    computeLogicalLeftPositionedOffset(computedValues.m_position, this, computedValues.m_extent + bordersPlusPadding, containerBlock, containerLogicalWidth, style().logicalLeft().isAuto(), style().logicalRight().isAuto());
 }
 
@@ -4674 +4675 @@
     LayoutUnit logicalLeftPos = logicalLeftValue + marginLogicalLeftAlias;
     // Border and padding have already been included in computedValues.m_extent.
-    computeLogicalLeftPositionedOffset(logicalLeftPos, this, computedValues.m_extent, containerBlock, containerLogicalWidth);
+    computeLogicalLeftPositionedOffset(logicalLeftPos, this, computedValues.m_extent, containerBlock, containerLogicalWidth, style().logicalLeft().isAuto(), style().logicalRight().isAuto());
     computedValues.m_position = logicalLeftPos;
 }