[CSS Grid Layout] Properly support for z-index on grid items
authorrego@igalia.com <rego@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Jun 2014 07:30:01 +0000 (07:30 +0000)
committerrego@igalia.com <rego@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Jun 2014 07:30:01 +0000 (07:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=103329

Reviewed by Benjamin Poulain.

From Blink r157620 by <jchaffraix@chromium.org> and r172402 by
<wangxianzhu@chromium.org>.

Source/WebCore:
The specification says that grid should match flexbox and force
grid items to be stacking context if z-index is set, regardless
of 'position'. After this change, this is what happens.

Previously z-index changes of static positioned objects were ignored.
However, z-index is applicable for some static positioned objects,
such as grid items. Ignoring them makes the object not to be properly
painted on z-index change.

As StyleResolver has ensured that z-index is non-auto only if
applicable, RenderStyle::changeRequiresLayerRepaint() should not check
again (with inconsistent conditions).

Tests: fast/css-grid-layout/grid-item-z-index-change-repaint.html
       fast/css-grid-layout/grid-item-z-index-stacking-context.html
       fast/css-grid-layout/grid-item-z-index-support.html

* css/StyleResolver.cpp:
(WebCore::isDisplayFlexibleOrGridBox): Add new method to check if parent
display is flexbox or grid.
(WebCore::StyleResolver::adjustRenderStyle): Use new method in z-index
condition.
* rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::changeRequiresLayerRepaint): Move z-index checks
out of non-static positioned objects if.

LayoutTests:
* css3/blending/repaint/blend-mode-turn-off-isolation-expected.txt: Test rebaseline.
* fast/css-grid-layout/grid-item-z-index-change-repaint-expected.html: Added.
* fast/css-grid-layout/grid-item-z-index-change-repaint.html: Added.
* fast/css-grid-layout/grid-item-z-index-stacking-context-expected.html: Added.
* fast/css-grid-layout/grid-item-z-index-stacking-context.html: Added.
* fast/css-grid-layout/grid-item-z-index-support-expected.txt: Added.
* fast/css-grid-layout/grid-item-z-index-support.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/css3/blending/repaint/blend-mode-turn-off-isolation-expected.txt
LayoutTests/fast/css-grid-layout/grid-item-z-index-change-repaint-expected.html [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-item-z-index-change-repaint.html [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-item-z-index-stacking-context-expected.html [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-item-z-index-stacking-context.html [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-item-z-index-support-expected.txt [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-item-z-index-support.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/StyleResolver.cpp
Source/WebCore/rendering/style/RenderStyle.cpp

index 04d2e15..0840574 100644 (file)
@@ -1,3 +1,21 @@
+2014-06-26  Manuel Rego Casasnovas  <rego@igalia.com>
+
+        [CSS Grid Layout] Properly support for z-index on grid items
+        https://bugs.webkit.org/show_bug.cgi?id=103329
+
+        Reviewed by Benjamin Poulain.
+
+        From Blink r157620 by <jchaffraix@chromium.org> and r172402 by
+        <wangxianzhu@chromium.org>.
+
+        * css3/blending/repaint/blend-mode-turn-off-isolation-expected.txt: Test rebaseline.
+        * fast/css-grid-layout/grid-item-z-index-change-repaint-expected.html: Added.
+        * fast/css-grid-layout/grid-item-z-index-change-repaint.html: Added.
+        * fast/css-grid-layout/grid-item-z-index-stacking-context-expected.html: Added.
+        * fast/css-grid-layout/grid-item-z-index-stacking-context.html: Added.
+        * fast/css-grid-layout/grid-item-z-index-support-expected.txt: Added.
+        * fast/css-grid-layout/grid-item-z-index-support.html: Added.
+
 2014-06-25  Myles C. Maxfield  <mmaxfield@apple.com>
 
         compositing/visible-rect/iframe-no-layers.html is broken and confusing
index 8a4ff04..c4b0828 100644 (file)
@@ -1,6 +1,9 @@
 This test checks that removing isolation from an element not being stacking context for other reasons will repaint the blending div.
 
 (repaint rects
+  (rect 8 68 100 100)
   (rect 58 68 100 100)
+  (rect 58 68 100 100)
+  (rect 8 68 100 100)
 )
 
diff --git a/LayoutTests/fast/css-grid-layout/grid-item-z-index-change-repaint-expected.html b/LayoutTests/fast/css-grid-layout/grid-item-z-index-change-repaint-expected.html
new file mode 100644 (file)
index 0000000..cda347a
--- /dev/null
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link href="resources/grid.css" rel="stylesheet">
+<style>
+.grid {
+    -webkit-grid-template-rows: 100px;
+    -webkit-grid-template-columns: 200px;
+    margin-top: 10px;
+}
+.green {
+    background-color: green;
+}
+</style>
+</head>
+<body>
+<div>
+    <p>This test checks that grid items correctly repaint when 'z-index' changes.</p>
+    <p>For this test to pass, there should be no red below.</p>
+</div>
+
+<div class="grid">
+    <div class="sizedToGridArea green"></div>
+</div>
+
+<div class="grid">
+    <div class="sizedToGridArea green"></div>
+</div>
+
+<div class="grid">
+    <div class="sizedToGridArea green"></div>
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/fast/css-grid-layout/grid-item-z-index-change-repaint.html b/LayoutTests/fast/css-grid-layout/grid-item-z-index-change-repaint.html
new file mode 100644 (file)
index 0000000..509e68c
--- /dev/null
@@ -0,0 +1,59 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link href="resources/grid.css" rel="stylesheet">
+<style>
+.grid {
+    -webkit-grid-template-rows: 100px;
+    -webkit-grid-template-columns: 200px;
+    margin-top: 10px;
+}
+.red {
+    background-color: red;
+}
+.green {
+    background-color: green;
+}
+.negativeZIndex {
+    z-index: -1;
+}
+#item3 {
+    width: 200px;
+    height: 100px;
+}
+</style>
+<script>
+function modifyZIndexAndDisplay()
+{
+    document.getElementById('item1').style.zIndex = 1;
+    document.getElementById('item2').style.zIndex = 1;
+
+    document.getElementById('grid3').className = "grid";
+}
+
+window.onload = modifyZIndexAndDisplay;
+</script>
+</head>
+<body>
+<div>
+    <p>This test checks that grid items correctly repaint when 'z-index' changes.</p>
+    <p>For this test to pass, there should be no red below.</p>
+</div>
+
+<div class="grid">
+    <div id="item1" class="sizedToGridArea green negativeZIndex"></div>
+    <div class="sizedToGridArea red"></div>
+</div>
+
+<div class="grid">
+    <div id="item2" class="sizedToGridArea green"></div>
+    <div class="sizedToGridArea red"></div>
+</div>
+
+<div id="grid3">
+    <div class="sizedToGridArea green"></div>
+    <div id="item3" class="red negativeZIndex"></div>
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/fast/css-grid-layout/grid-item-z-index-stacking-context-expected.html b/LayoutTests/fast/css-grid-layout/grid-item-z-index-stacking-context-expected.html
new file mode 100644 (file)
index 0000000..bda0fc0
--- /dev/null
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link href="resources/grid.css" rel="stylesheet">
+<style>
+.grid {
+    -webkit-grid-template-rows: 100px;
+    -webkit-grid-template-columns: 200px 200px;
+    margin-top: 10px;
+}
+.green {
+    background-color: green;
+}
+</style>
+</head>
+<body>
+<p>This test checks that grid items with 'z-index' do produce a stacking context and that we honor the property.</p>
+<p>For this test to pass, there should be no red below.</p>
+
+<div class="grid">
+    <div class="sizedToGridArea green"></div>
+</div>
+
+<div class="grid">
+    <div class="sizedToGridArea green"></div>
+</div>
+
+<div class="grid">
+    <div class="sizedToGridArea green firstRowBothColumn"></div>
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/fast/css-grid-layout/grid-item-z-index-stacking-context.html b/LayoutTests/fast/css-grid-layout/grid-item-z-index-stacking-context.html
new file mode 100644 (file)
index 0000000..5984f53
--- /dev/null
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link href="resources/grid.css" rel="stylesheet">
+<style>
+.grid {
+    -webkit-grid-template-rows: 100px;
+    -webkit-grid-template-columns: 200px 200px;
+    margin-top: 10px;
+}
+.red {
+    background-color: red;
+}
+.green {
+    background-color: green;
+}
+.positiveZIndex {
+    z-index: 10;
+}
+.negativeZIndex {
+    z-index: -5;
+}
+</style>
+</head>
+<body>
+<p>This test checks that grid items with 'z-index' do produce a stacking context and that we honor the property.</p>
+<p>For this test to pass, there should be no red below.</p>
+
+<div class="grid">
+    <div class="sizedToGridArea green positiveZIndex"></div>
+    <div class="sizedToGridArea red"></div>
+</div>
+
+<div class="grid">
+    <div class="sizedToGridArea green"></div>
+    <div class="sizedToGridArea red negativeZIndex"></div>
+</div>
+
+<div class="grid">
+    <div class="sizedToGridArea green firstRowBothColumn"></div>
+    <div class="sizedToGridArea red negativeZIndex firstRowFirstColumn"></div>
+    <div class="sizedToGridArea red negativeZIndex firstRowSecondColumn"></div>
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/fast/css-grid-layout/grid-item-z-index-support-expected.txt b/LayoutTests/fast/css-grid-layout/grid-item-z-index-support-expected.txt
new file mode 100644 (file)
index 0000000..8a27e8d
--- /dev/null
@@ -0,0 +1,15 @@
+Test that an element supports z-index once it is a grid item and it is repainted properly.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Test z-index property for a regular element
+PASS getComputedStyle(item, '').getPropertyValue('z-index') is 'auto'
+PASS getComputedStyle(item, '').getPropertyValue('z-index') is 'auto'
+Test z-index property once the element becomes a grid item
+PASS getComputedStyle(item, '').getPropertyValue('z-index') is '-10'
+PASS getComputedStyle(item, '').getPropertyValue('z-index') is '10'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/css-grid-layout/grid-item-z-index-support.html b/LayoutTests/fast/css-grid-layout/grid-item-z-index-support.html
new file mode 100644 (file)
index 0000000..4c4cc04
--- /dev/null
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+</head>
+<body>
+<div id="grid">
+    <div id="item"></div>
+</div>
+<script>
+description('Test that an element supports z-index once it is a grid item and it is repainted properly.');
+
+debug('Test z-index property for a regular element');
+var item = document.getElementById("item");
+shouldBe("getComputedStyle(item, '').getPropertyValue('z-index')", "'auto'");
+item.style.zIndex = "-10";
+shouldBe("getComputedStyle(item, '').getPropertyValue('z-index')", "'auto'");
+
+debug('Test z-index property once the element becomes a grid item');
+var grid = document.getElementById("grid");
+grid.style.display = "-webkit-grid";
+shouldBe("getComputedStyle(item, '').getPropertyValue('z-index')", "'-10'");
+item.style.zIndex = "10";
+shouldBe("getComputedStyle(item, '').getPropertyValue('z-index')", "'10'");
+
+</script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
index 9e4d7c9..7d730b5 100644 (file)
@@ -1,3 +1,39 @@
+2014-06-26  Manuel Rego Casasnovas  <rego@igalia.com>
+
+        [CSS Grid Layout] Properly support for z-index on grid items
+        https://bugs.webkit.org/show_bug.cgi?id=103329
+
+        Reviewed by Benjamin Poulain.
+
+        From Blink r157620 by <jchaffraix@chromium.org> and r172402 by
+        <wangxianzhu@chromium.org>.
+
+        The specification says that grid should match flexbox and force
+        grid items to be stacking context if z-index is set, regardless
+        of 'position'. After this change, this is what happens.
+
+        Previously z-index changes of static positioned objects were ignored.
+        However, z-index is applicable for some static positioned objects,
+        such as grid items. Ignoring them makes the object not to be properly
+        painted on z-index change.
+
+        As StyleResolver has ensured that z-index is non-auto only if
+        applicable, RenderStyle::changeRequiresLayerRepaint() should not check
+        again (with inconsistent conditions).
+
+        Tests: fast/css-grid-layout/grid-item-z-index-change-repaint.html
+               fast/css-grid-layout/grid-item-z-index-stacking-context.html
+               fast/css-grid-layout/grid-item-z-index-support.html
+
+        * css/StyleResolver.cpp:
+        (WebCore::isDisplayFlexibleOrGridBox): Add new method to check if parent
+        display is flexbox or grid.
+        (WebCore::StyleResolver::adjustRenderStyle): Use new method in z-index
+        condition.
+        * rendering/style/RenderStyle.cpp:
+        (WebCore::RenderStyle::changeRequiresLayerRepaint): Move z-index checks
+        out of non-static positioned objects if.
+
 2014-06-25  Dean Jackson  <dino@apple.com>
 
         HIDGamepad should use CFIndex when looping
index 598a990..5bd3471 100644 (file)
@@ -1136,6 +1136,11 @@ static inline bool isDisplayGridBox(EDisplay display)
 #endif
 }
 
+static bool isDisplayFlexibleOrGridBox(EDisplay display)
+{
+    return isDisplayFlexibleBox(display) || isDisplayGridBox(display);
+}
+
 #if ENABLE(ACCELERATED_OVERFLOW_SCROLLING)
 static bool isScrollableOverflow(EOverflow overflow)
 {
@@ -1228,14 +1233,14 @@ void StyleResolver::adjustRenderStyle(RenderStyle& style, const RenderStyle& par
         if (style.writingMode() != TopToBottomWritingMode && (style.display() == BOX || style.display() == INLINE_BOX))
             style.setWritingMode(TopToBottomWritingMode);
 
-        if (isDisplayFlexibleBox(parentStyle.display()) || isDisplayGridBox(parentStyle.display())) {
+        if (isDisplayFlexibleOrGridBox(parentStyle.display())) {
             style.setFloating(NoFloat);
             style.setDisplay(equivalentBlockDisplay(style.display(), style.isFloating(), !document().inQuirksMode()));
         }
     }
 
     // Make sure our z-index value is only applied if the object is positioned.
-    if (style.position() == StaticPosition && !isDisplayFlexibleBox(parentStyle.display()))
+    if (style.position() == StaticPosition && !isDisplayFlexibleOrGridBox(parentStyle.display()))
         style.setHasAutoZIndex();
 
     // Auto z-index becomes 0 for the root element and transparent objects. This prevents
index 0877386..91b3151 100644 (file)
@@ -673,10 +673,12 @@ bool RenderStyle::changeRequiresPositionedLayoutOnly(const RenderStyle* other, u
 
 bool RenderStyle::changeRequiresLayerRepaint(const RenderStyle* other, unsigned& changedContextSensitiveProperties) const
 {
+    // StyleResolver has ensured that zIndex is non-auto only if it's applicable.
+    if (m_box->zIndex() != other->m_box->zIndex() || m_box->hasAutoZIndex() != other->m_box->hasAutoZIndex())
+        return true;
+
     if (position() != StaticPosition) {
-        if (m_box->zIndex() != other->m_box->zIndex()
-            || m_box->hasAutoZIndex() != other->m_box->hasAutoZIndex()
-            || visual->clip != other->visual->clip
+        if (visual->clip != other->visual->clip
             || visual->hasClip != other->visual->hasClip)
             return true;
     }