[css-grid] Fix percentages in relative offsets for grid items
authorrego@igalia.com <rego@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Dec 2018 12:29:57 +0000 (12:29 +0000)
committerrego@igalia.com <rego@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Dec 2018 12:29:57 +0000 (12:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=190492

Reviewed by Sergio Villar Senin.

LayoutTests/imported/w3c:

* web-platform-tests/css/css-grid/grid-items/grid-items-relative-offsets-002-expected.txt:
  Update expected file as we're now passing this test.

Source/WebCore:

The method RenderBoxModelObject::relativePositionOffset() was not considering the case of grid items,
where the containing block is the grid area.
The patch modifies the method so the new code uses overrideContainingBlockContentWidth|Height when required.

Test: imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-relative-offsets-002.html

* rendering/RenderBox.cpp: Implement the physical versions of the already existent methods.
(WebCore::RenderBox::overrideContainingBlockContentWidth const):
(WebCore::RenderBox::overrideContainingBlockContentHeight const):
(WebCore::RenderBox::hasOverrideContainingBlockContentWidth const):
(WebCore::RenderBox::hasOverrideContainingBlockContentHeight const):
* rendering/RenderBox.h:
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::relativePositionOffset const): Modified method
to take into account overrideContainingBlockContentWidth|Height for grid items.
* rendering/RenderBoxModelObject.h: Added new headers for physical virtual methods
that will be overridden in RenderBox.
(WebCore::RenderBoxModelObject::overrideContainingBlockContentWidth const):
(WebCore::RenderBoxModelObject::overrideContainingBlockContentHeight const):
(WebCore::RenderBoxModelObject::hasOverrideContainingBlockContentWidth const):
(WebCore::RenderBoxModelObject::hasOverrideContainingBlockContentHeight const):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239502 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-relative-offsets-002-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBox.cpp
Source/WebCore/rendering/RenderBox.h
Source/WebCore/rendering/RenderBoxModelObject.cpp
Source/WebCore/rendering/RenderBoxModelObject.h

index 995fecb..be7c47b 100644 (file)
@@ -1,3 +1,13 @@
+2018-12-21  Manuel Rego Casasnovas  <rego@igalia.com>
+
+        [css-grid] Fix percentages in relative offsets for grid items
+        https://bugs.webkit.org/show_bug.cgi?id=190492
+
+        Reviewed by Sergio Villar Senin.
+
+        * web-platform-tests/css/css-grid/grid-items/grid-items-relative-offsets-002-expected.txt:
+          Update expected file as we're now passing this test.
+
 2018-12-19  Youenn Fablet  <youenn@apple.com>
 
         Remove RTCRtpTransceiver.setDirection
index d4825b5..368e0b7 100644 (file)
@@ -1,46 +1,10 @@
 
-FAIL .grid 1 assert_equals: 
-<div class="grid verticalRL directionRTL">
-  <div class="firstRowFirstColumn" style="left: 10%; top: 5%;" data-offset-x="219" data-offest-y="10" data-expected-width="90" data-expected-height="200"></div>
-  <div class="secondRowSecondColumn" style="left: -20%; top: -10%;" data-offset-x="138" data-offest-y="75" data-expected-width="60" data-expected-height="150"></div>
-  <div class="thirdRowThirdColumn" style="right: 70%; bottom: 30%;" data-offset-x="99" data-offest-y="120" data-expected-width="30" data-expected-height="100"></div>
-</div>
-offsetLeft expected 219 but got 240
-FAIL .grid 2 assert_equals: 
-<div class="grid verticalRL">
-  <div class="firstRowFirstColumn" style="left: 10%; top: 5%;" data-offset-x="219" data-offest-y="10" data-expected-width="90" data-expected-height="200"></div>
-  <div class="secondRowSecondColumn" style="left: -20%; top: -10%;" data-offset-x="138" data-offest-y="75" data-expected-width="60" data-expected-height="150"></div>
-  <div class="thirdRowThirdColumn" style="right: 70%; bottom: 30%;" data-offset-x="99" data-offest-y="120" data-expected-width="30" data-expected-height="100"></div>
-</div>
-offsetLeft expected 219 but got 240
-FAIL .grid 3 assert_equals: 
-<div class="grid verticalLR directionRTL">
-  <div class="firstRowFirstColumn" style="left: 10%; top: 5%;" data-offset-x="9" data-offest-y="10" data-expected-width="90" data-expected-height="200"></div>
-  <div class="secondRowSecondColumn" style="left: -20%; top: -10%;" data-offset-x="78" data-offest-y="75" data-expected-width="60" data-expected-height="150"></div>
-  <div class="thirdRowThirdColumn" style="right: 70%; bottom: 30%;" data-offset-x="129" data-offest-y="120" data-expected-width="30" data-expected-height="100"></div>
-</div>
-offsetLeft expected 9 but got 30
-FAIL .grid 4 assert_equals: 
-<div class="grid verticalLR">
-  <div class="firstRowFirstColumn" style="left: 10%; top: 5%;" data-offset-x="9" data-offest-y="10" data-expected-width="90" data-expected-height="200"></div>
-  <div class="secondRowSecondColumn" style="left: -20%; top: -10%;" data-offset-x="78" data-offest-y="75" data-expected-width="60" data-expected-height="150"></div>
-  <div class="thirdRowThirdColumn" style="right: 70%; bottom: 30%;" data-offset-x="129" data-offest-y="120" data-expected-width="30" data-expected-height="100"></div>
-</div>
-offsetLeft expected 9 but got 30
-FAIL .grid 5 assert_equals: 
-<div class="grid directionRTL">
-  <div class="firstRowFirstColumn" style="left: 5%; top: 10%;" data-offset-x="410" data-offest-y="9" data-expected-width="200" data-expected-height="90"></div>
-  <div class="secondRowSecondColumn" style="left: -10%; top: -20%;" data-offset-x="235" data-offest-y="78" data-expected-width="150" data-expected-height="60"></div>
-  <div class="thirdRowThirdColumn" style="right: 30%; bottom: 70%;" data-offset-x="120" data-offest-y="129" data-expected-width="100" data-expected-height="30"></div>
-</div>
-offsetLeft expected 410 but got 430
-FAIL .grid 6 assert_equals: 
-<div class="grid">
-  <div class="firstRowFirstColumn" style="left: 5%; top: 10%;" data-offset-x="10" data-offest-y="9" data-expected-width="200" data-expected-height="90"></div>
-  <div class="secondRowSecondColumn" style="left: -10%; top: -20%;" data-offset-x="185" data-offest-y="78" data-expected-width="150" data-expected-height="60"></div>
-  <div class="thirdRowThirdColumn" style="right: 30%; bottom: 70%;" data-offset-x="320" data-offest-y="129" data-expected-width="100" data-expected-height="30"></div>
-</div>
-offsetLeft expected 10 but got 30
+PASS .grid 1 
+PASS .grid 2 
+PASS .grid 3 
+PASS .grid 4 
+PASS .grid 5 
+PASS .grid 6 
 Direction LTR
 
 Direction RTL
index 1675be7..236fedc 100644 (file)
@@ -1,3 +1,32 @@
+2018-12-21  Manuel Rego Casasnovas  <rego@igalia.com>
+
+        [css-grid] Fix percentages in relative offsets for grid items
+        https://bugs.webkit.org/show_bug.cgi?id=190492
+
+        Reviewed by Sergio Villar Senin.
+
+        The method RenderBoxModelObject::relativePositionOffset() was not considering the case of grid items,
+        where the containing block is the grid area.
+        The patch modifies the method so the new code uses overrideContainingBlockContentWidth|Height when required.
+
+        Test: imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-relative-offsets-002.html
+
+        * rendering/RenderBox.cpp: Implement the physical versions of the already existent methods.
+        (WebCore::RenderBox::overrideContainingBlockContentWidth const):
+        (WebCore::RenderBox::overrideContainingBlockContentHeight const):
+        (WebCore::RenderBox::hasOverrideContainingBlockContentWidth const):
+        (WebCore::RenderBox::hasOverrideContainingBlockContentHeight const):
+        * rendering/RenderBox.h:
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::relativePositionOffset const): Modified method
+        to take into account overrideContainingBlockContentWidth|Height for grid items.
+        * rendering/RenderBoxModelObject.h: Added new headers for physical virtual methods
+        that will be overridden in RenderBox.
+        (WebCore::RenderBoxModelObject::overrideContainingBlockContentWidth const):
+        (WebCore::RenderBoxModelObject::overrideContainingBlockContentHeight const):
+        (WebCore::RenderBoxModelObject::hasOverrideContainingBlockContentWidth const):
+        (WebCore::RenderBoxModelObject::hasOverrideContainingBlockContentHeight const):
+
 2018-12-20  Justin Fan  <justin_fan@apple.com>
 
         [WebGPU] Convert WebGPUBindGroups into MTLArgumentEncoders
index a671111..06aebaf 100644 (file)
@@ -98,6 +98,7 @@ static OverrideSizeMap* gOverrideContentLogicalHeightMap = nullptr;
 static OverrideSizeMap* gOverrideContentLogicalWidthMap = nullptr;
 
 // Used by grid elements to properly size their grid items.
+// FIXME: We should store these based on physical direction.
 typedef WTF::HashMap<const RenderBox*, Optional<LayoutUnit>> OverrideOptionalSizeMap;
 static OverrideOptionalSizeMap* gOverrideContainingBlockContentLogicalHeightMap = nullptr;
 static OverrideOptionalSizeMap* gOverrideContainingBlockContentLogicalWidthMap = nullptr;
@@ -1086,6 +1087,44 @@ LayoutUnit RenderBox::overrideContentLogicalHeight() const
     return gOverrideContentLogicalHeightMap->get(this);
 }
 
+Optional<LayoutUnit> RenderBox::overrideContainingBlockContentWidth() const
+{
+    ASSERT(hasOverrideContainingBlockContentWidth());
+    return containingBlock()->style().isHorizontalWritingMode()
+        ? gOverrideContainingBlockContentLogicalWidthMap->get(this)
+        : gOverrideContainingBlockContentLogicalHeightMap->get(this);
+}
+
+Optional<LayoutUnit> RenderBox::overrideContainingBlockContentHeight() const
+{
+    ASSERT(hasOverrideContainingBlockContentHeight());
+    return containingBlock()->style().isHorizontalWritingMode()
+        ? gOverrideContainingBlockContentLogicalHeightMap->get(this)
+        : gOverrideContainingBlockContentLogicalWidthMap->get(this);
+}
+
+bool RenderBox::hasOverrideContainingBlockContentWidth() const
+{
+    RenderBlock* cb = containingBlock();
+    if (!cb)
+        return false;
+
+    return cb->style().isHorizontalWritingMode()
+        ? gOverrideContainingBlockContentLogicalWidthMap && gOverrideContainingBlockContentLogicalWidthMap->contains(this)
+        : gOverrideContainingBlockContentLogicalHeightMap && gOverrideContainingBlockContentLogicalHeightMap->contains(this);
+}
+
+bool RenderBox::hasOverrideContainingBlockContentHeight() const
+{
+    RenderBlock* cb = containingBlock();
+    if (!cb)
+        return false;
+
+    return cb->style().isHorizontalWritingMode()
+        ? gOverrideContainingBlockContentLogicalHeightMap && gOverrideContainingBlockContentLogicalHeightMap->contains(this)
+        : gOverrideContainingBlockContentLogicalHeightMap && gOverrideContainingBlockContentLogicalHeightMap->contains(this);
+}
+
 Optional<LayoutUnit> RenderBox::overrideContainingBlockContentLogicalWidth() const
 {
     ASSERT(hasOverrideContainingBlockContentLogicalWidth());
index 88d9171..5510685 100644 (file)
@@ -314,6 +314,10 @@ public:
     void clearOverrideContentLogicalHeight();
     void clearOverrideContentLogicalWidth();
 
+    Optional<LayoutUnit> overrideContainingBlockContentWidth() const override;
+    Optional<LayoutUnit> overrideContainingBlockContentHeight() const override;
+    bool hasOverrideContainingBlockContentWidth() const override;
+    bool hasOverrideContainingBlockContentHeight() const override;
     Optional<LayoutUnit> overrideContainingBlockContentLogicalWidth() const;
     Optional<LayoutUnit> overrideContainingBlockContentLogicalHeight() const;
     bool hasOverrideContainingBlockContentLogicalWidth() const;
index 96052ea..fdc511c 100644 (file)
@@ -401,13 +401,18 @@ LayoutSize RenderBoxModelObject::relativePositionOffset() const
     // in the case of relative positioning using percentages, we can't do this.  The offset should always be resolved using the
     // available width of the containing block.  Therefore we don't use containingBlockLogicalWidthForContent() here, but instead explicitly
     // call availableWidth on our containing block.
-    if (!style().left().isAuto()) {
-        if (!style().right().isAuto() && !containingBlock()->style().isLeftToRightDirection())
-            offset.setWidth(-valueForLength(style().right(), !style().right().isFixed() ? containingBlock()->availableWidth() : 0_lu));
-        else
-            offset.expand(valueForLength(style().left(), !style().left().isFixed() ? containingBlock()->availableWidth() : 0_lu), 0_lu);
-    } else if (!style().right().isAuto()) {
-        offset.expand(-valueForLength(style().right(), !style().right().isFixed() ? containingBlock()->availableWidth() : 0_lu), 0_lu);
+    // However for grid items the containing block is the grid area, so offsets should be resolved against that:
+    // https://drafts.csswg.org/css-grid/#grid-item-sizing
+    if (!style().left().isAuto() || !style().right().isAuto()) {
+        LayoutUnit availableWidth = hasOverrideContainingBlockContentWidth()
+            ? overrideContainingBlockContentWidth().valueOr(LayoutUnit()) : containingBlock()->availableWidth();
+        if (!style().left().isAuto()) {
+            if (!style().right().isAuto() && !containingBlock()->style().isLeftToRightDirection())
+                offset.setWidth(-valueForLength(style().right(), !style().right().isFixed() ? availableWidth : 0_lu));
+            else
+                offset.expand(valueForLength(style().left(), !style().left().isFixed() ? availableWidth : 0_lu), 0_lu);
+        } else if (!style().right().isAuto())
+            offset.expand(-valueForLength(style().right(), !style().right().isFixed() ? availableWidth : 0_lu), 0_lu);
     }
 
     // If the containing block of a relatively positioned element does not
@@ -416,17 +421,29 @@ LayoutSize RenderBoxModelObject::relativePositionOffset() const
     // where <html> and <body> assume the size of the viewport. In this case,
     // calculate the percent offset based on this height.
     // See <https://bugs.webkit.org/show_bug.cgi?id=26396>.
+    // Another exception is a grid item, as the containing block is the grid area:
+    // https://drafts.csswg.org/css-grid/#grid-item-sizing
     if (!style().top().isAuto()
         && (!style().top().isPercentOrCalculated()
             || !containingBlock()->hasAutoHeightOrContainingBlockWithAutoHeight()
-            || containingBlock()->stretchesToViewport()))
-        offset.expand(0_lu, valueForLength(style().top(), !style().top().isFixed() ? containingBlock()->availableHeight() : 0_lu));
-
-    else if (!style().bottom().isAuto()
+            || containingBlock()->stretchesToViewport()
+            || hasOverrideContainingBlockContentHeight())) {
+        // FIXME: The computation of the available height is repeated later for "bottom".
+        // We could refactor this and move it to some common code for both ifs, however moving it outside of the ifs
+        // is not possible as it'd cause performance regressions.
+        offset.expand(0_lu, valueForLength(style().top(), !style().top().isFixed()
+            ? (hasOverrideContainingBlockContentHeight() ? overrideContainingBlockContentHeight().valueOr(0_lu) : containingBlock()->availableHeight())
+            : LayoutUnit()));
+    } else if (!style().bottom().isAuto()
         && (!style().bottom().isPercentOrCalculated()
             || !containingBlock()->hasAutoHeightOrContainingBlockWithAutoHeight()
-            || containingBlock()->stretchesToViewport()))
-        offset.expand(0_lu, -valueForLength(style().bottom(), !style().bottom().isFixed() ? containingBlock()->availableHeight() : 0_lu));
+            || containingBlock()->stretchesToViewport()
+            || hasOverrideContainingBlockContentHeight())) {
+        // FIXME: Check comment above for "top", it applies here too.
+        offset.expand(0_lu, -valueForLength(style().bottom(), !style().bottom().isFixed()
+            ? (hasOverrideContainingBlockContentHeight() ? overrideContainingBlockContentHeight().valueOr(0_lu) : containingBlock()->availableHeight())
+            : LayoutUnit()));
+    }
 
     return offset;
 }
index 94b69a1..86f4b80 100644 (file)
@@ -243,7 +243,12 @@ public:
     virtual LayoutRect paintRectToClipOutFromBorder(const LayoutRect&) { return LayoutRect(); };
 
     bool hasRunningAcceleratedAnimations() const;
-    
+
+    virtual Optional<LayoutUnit> overrideContainingBlockContentWidth() const { ASSERT_NOT_REACHED(); return -1_lu; }
+    virtual Optional<LayoutUnit> overrideContainingBlockContentHeight() const { ASSERT_NOT_REACHED(); return -1_lu; }
+    virtual bool hasOverrideContainingBlockContentWidth() const { return false; }
+    virtual bool hasOverrideContainingBlockContentHeight() const { return false; }
+
 protected:
     RenderBoxModelObject(Element&, RenderStyle&&, BaseTypeFlags);
     RenderBoxModelObject(Document&, RenderStyle&&, BaseTypeFlags);