[CSS Grid Layout] Ignore ::first-letter pseudo-element
authorrego@igalia.com <rego@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 9 Sep 2014 21:58:11 +0000 (21:58 +0000)
committerrego@igalia.com <rego@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 9 Sep 2014 21:58:11 +0000 (21:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=136625

Reviewed by Darin Adler.

Source/WebCore:

According to the spec the ::first-letter pseudo-element do not apply to
grid containers (neither to flexboxes).

Fixed issue in RenderBlock::getFirstLetter() that applies to both grids
and flexboxes. Basically if the grid's or flexbox's container was
defining the ::first-line pseudo-element and the grid or flexbox itself
too, the value from the grid or flexbox was being applied to the items.
Added the proper check to avoid this.

Added two new tests for grid and modified one flexbox test to cover the
issue explained above.

Tests: css3/flexbox/flexbox-ignore-container-firstLetter.html
       fast/css-grid-layout/grid-container-ignore-first-letter.html
       fast/css-grid-layout/grid-item-first-letter-valid.html

* rendering/RenderBlock.cpp:
(WebCore::isRenderBlockFlowOrRenderButton): New method refactoring
similar calls through the source code.
(WebCore::RenderBlock::firstLineBlock): Use
isRenderBlockFlowOrRenderButton().
(WebCore::findFirstLetterBlock): Modify it to use
isRenderBlockFlowOrRenderButton() in order to include grids and not only
check flexboxes.
(WebCore::RenderBlock::getFirstLetter): Use
isRenderBlockFlowOrRenderButton().
(WebCore::RenderBlock::updateFirstLetter): Early return if
firstLetterContainer is null (in the case of flexboxes or grids).

LayoutTests:

* css3/flexbox/flexbox-ignore-container-firstLetter.html: Modify test,
in order to actually check that the ::first-letter in a regular
container is ignored in the flexbox.
* fast/css-grid-layout/grid-container-ignore-first-letter-expected.txt: Added.
* fast/css-grid-layout/grid-container-ignore-first-letter.html: Added.
* fast/css-grid-layout/grid-item-first-letter-valid-expected.txt: Added.
* fast/css-grid-layout/grid-item-first-letter-valid.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter.html
LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter-expected.txt [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter.html [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid-expected.txt [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp

index d724bfc..8e1ccf4 100644 (file)
@@ -1,3 +1,18 @@
+2014-09-09  Manuel Rego Casasnovas  <rego@igalia.com>
+
+        [CSS Grid Layout] Ignore ::first-letter pseudo-element
+        https://bugs.webkit.org/show_bug.cgi?id=136625
+
+        Reviewed by Darin Adler.
+
+        * css3/flexbox/flexbox-ignore-container-firstLetter.html: Modify test,
+        in order to actually check that the ::first-letter in a regular
+        container is ignored in the flexbox.
+        * fast/css-grid-layout/grid-container-ignore-first-letter-expected.txt: Added.
+        * fast/css-grid-layout/grid-container-ignore-first-letter.html: Added.
+        * fast/css-grid-layout/grid-item-first-letter-valid-expected.txt: Added.
+        * fast/css-grid-layout/grid-item-first-letter-valid.html: Added.
+
 2014-09-09  Benjamin Poulain  <benjamin@webkit.org>
 
         Add support for :read-write/:read-only matching editable content
index f30b9b4..48aa5bb 100644 (file)
@@ -2,8 +2,8 @@
 <html>
 <link href="resources/flexbox.css" rel="stylesheet">
 <style>
-    .container { display: -webkit-flex }
     .container::first-letter { line-height: 100px; }
+    .flexbox::first-letter { line-height: 200px; }
     p { line-height: 20px; }
 </style>
 
diff --git a/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter-expected.txt b/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter-expected.txt
new file mode 100644 (file)
index 0000000..8f9afac
--- /dev/null
@@ -0,0 +1,22 @@
+This test grid item should ignore grid container's first-letter pseudo-element.
+
+The first item.
+The second item.
+PASS
+The first item.
+The second item.
+PASS
+Anonymous item.
+PASS
+Anonymous item.
+PASS
+The first item.
+The second item.
+PASS
+The first item.
+The second item.
+ PASS
+Anonymous item.
+PASS
+Anonymous item.
+PASS
diff --git a/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter.html b/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter.html
new file mode 100644 (file)
index 0000000..15b2f43
--- /dev/null
@@ -0,0 +1,59 @@
+<!DOCTYPE html>
+<html>
+<link href="resources/grid.css" rel="stylesheet">
+<style>
+    body { line-height: 20px; }
+    .grid-first-letter::first-letter { line-height: 100px; color: red; }
+    .container::first-letter { line-height: 200px; color: green; }
+</style>
+
+<script src="../../resources/check-layout.js"></script>
+<body onload="checkLayout('.grid-first-letter');">
+<p>This test grid item should ignore grid container's first-letter pseudo-element.</p>
+
+<div class="grid grid-first-letter">
+    <div class="item" data-expected-height=20>The first item.</div>
+    <div class="item" data-expected-height=20>The second item.</div>
+</div>
+
+<div class="inline-grid grid-first-letter">
+    <div class="item" data-expected-height=20>The first item.</div>
+    <div class="item" data-expected-height=20>The second item.</div>
+</div>
+
+<div class="grid grid-first-letter" data-expected-height=20>
+    Anonymous item.
+</div>
+
+<div class="inline-grid grid-first-letter" data-expected-height=20>
+    Anonymous item.
+</div>
+
+<div class="container">
+    <div class="grid grid-first-letter">
+        <div class="item" data-expected-height=20>The first item.</div>
+        <div class="item" data-expected-height=20>The second item.</div>
+    </div>
+</div>
+
+<div class="container">
+    <div class="inline-grid grid-first-letter">
+        <div class="item" data-expected-height=20>The first item.</div>
+        <div class="item" data-expected-height=20>The second item.</div>
+    </div>
+</div>
+
+<div class="container">
+    <div class="grid grid-first-letter" data-expected-height=20>
+        Anonymous item.
+    </div>
+</div>
+
+<div class="container">
+    <div class="inline-grid grid-first-letter" data-expected-height=20>
+        Anonymous item.
+    </div>
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid-expected.txt b/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid-expected.txt
new file mode 100644 (file)
index 0000000..6e8bae5
--- /dev/null
@@ -0,0 +1,8 @@
+This test grid item's first-letter pseudo-element should be valid.
+
+The first item.
+The second item.
+PASS
+The first item.
+The second item.
+PASS
diff --git a/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid.html b/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid.html
new file mode 100644 (file)
index 0000000..b980093
--- /dev/null
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<link href="resources/grid.css" rel="stylesheet">
+<style>
+    .item::first-letter { line-height: 100px; }
+</style>
+
+<script src="../../resources/check-layout.js"></script>
+<body onload="checkLayout('.grid'); checkLayout('.inline-grid');">
+<p>This test grid item's first-letter pseudo-element should be valid.</p>
+
+<div class="grid">
+    <div class="item" data-expected-height=100>The first item.</div>
+    <div class="item" data-expected-height=100>The second item.</div>
+</div>
+
+<div class="inline-grid">
+    <div class="item" data-expected-height=100>The first item.</div>
+    <div class="item" data-expected-height=100>The second item.</div>
+</div>
+
+</body>
+</html>
index eb6a2ce..c0138ae 100644 (file)
@@ -1,3 +1,39 @@
+2014-09-09  Manuel Rego Casasnovas  <rego@igalia.com>
+
+        [CSS Grid Layout] Ignore ::first-letter pseudo-element
+        https://bugs.webkit.org/show_bug.cgi?id=136625
+
+        Reviewed by Darin Adler.
+
+        According to the spec the ::first-letter pseudo-element do not apply to
+        grid containers (neither to flexboxes).
+
+        Fixed issue in RenderBlock::getFirstLetter() that applies to both grids
+        and flexboxes. Basically if the grid's or flexbox's container was
+        defining the ::first-line pseudo-element and the grid or flexbox itself
+        too, the value from the grid or flexbox was being applied to the items.
+        Added the proper check to avoid this.
+
+        Added two new tests for grid and modified one flexbox test to cover the
+        issue explained above.
+
+        Tests: css3/flexbox/flexbox-ignore-container-firstLetter.html
+               fast/css-grid-layout/grid-container-ignore-first-letter.html
+               fast/css-grid-layout/grid-item-first-letter-valid.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::isRenderBlockFlowOrRenderButton): New method refactoring
+        similar calls through the source code.
+        (WebCore::RenderBlock::firstLineBlock): Use
+        isRenderBlockFlowOrRenderButton().
+        (WebCore::findFirstLetterBlock): Modify it to use
+        isRenderBlockFlowOrRenderButton() in order to include grids and not only
+        check flexboxes.
+        (WebCore::RenderBlock::getFirstLetter): Use
+        isRenderBlockFlowOrRenderButton().
+        (WebCore::RenderBlock::updateFirstLetter): Early return if
+        firstLetterContainer is null (in the case of flexboxes or grids).
+
 2014-09-09  Benjamin Poulain  <benjamin@webkit.org>
 
         Add support for :read-write/:read-only matching editable content
index 6bbe4be..1f31242 100644 (file)
@@ -3019,6 +3019,17 @@ int RenderBlock::inlineBlockBaseline(LineDirectionMode lineDirection) const
     return -1;
 }
 
+static inline bool isRenderBlockFlowOrRenderButton(RenderElement& renderElement)
+{
+    // We include isRenderButton in this check because buttons are implemented
+    // using flex box but should still support first-line|first-letter.
+    // The flex box and specs require that flex box and grid do not support
+    // first-line|first-letter, though.
+    // FIXME: Remove when buttons are implemented with align-items instead of
+    // flex box.
+    return renderElement.isRenderBlockFlow() || renderElement.isRenderButton();
+}
+
 RenderBlock* RenderBlock::firstLineBlock() const
 {
     RenderBlock* firstLineBlock = const_cast<RenderBlock*>(this);
@@ -3028,14 +3039,8 @@ RenderBlock* RenderBlock::firstLineBlock() const
         if (hasPseudo)
             break;
         RenderElement* parentBlock = firstLineBlock->parent();
-        // We include isRenderButton in this check because buttons are
-        // implemented using flex box but should still support first-line. The
-        // flex box spec requires that flex box does not support first-line,
-        // though.
-        // FIXME: Remove when buttons are implemented with align-items instead
-        // of flexbox.
         if (firstLineBlock->isReplaced() || firstLineBlock->isFloating()
-            || !parentBlock || parentBlock->firstChild() != firstLineBlock || (!parentBlock->isRenderBlockFlow() && !parentBlock->isRenderButton()))
+            || !parentBlock || parentBlock->firstChild() != firstLineBlock || !isRenderBlockFlowOrRenderButton(*parentBlock))
             break;
         firstLineBlock = toRenderBlock(parentBlock);
     } 
@@ -3111,21 +3116,15 @@ static inline RenderBlock* findFirstLetterBlock(RenderBlock* start)
 {
     RenderBlock* firstLetterBlock = start;
     while (true) {
-        // We include isRenderButton in these two checks because buttons are
-        // implemented using flex box but should still support first-letter.
-        // The flex box spec requires that flex box does not support
-        // first-letter, though.
-        // FIXME: Remove when buttons are implemented with align-items instead
-        // of flexbox.
         bool canHaveFirstLetterRenderer = firstLetterBlock->style().hasPseudoStyle(FIRST_LETTER)
             && firstLetterBlock->canHaveGeneratedChildren()
-            && (!firstLetterBlock->isFlexibleBox() || firstLetterBlock->isRenderButton());
+            && isRenderBlockFlowOrRenderButton(*firstLetterBlock);
         if (canHaveFirstLetterRenderer)
             return firstLetterBlock;
 
         RenderElement* parentBlock = firstLetterBlock->parent();
         if (firstLetterBlock->isReplaced() || !parentBlock || parentBlock->firstChild() != firstLetterBlock
-            || (!parentBlock->isRenderBlockFlow() && !parentBlock->isRenderButton()))
+            || !isRenderBlockFlowOrRenderButton(*parentBlock))
             return 0;
         firstLetterBlock = toRenderBlock(parentBlock);
     } 
@@ -3293,7 +3292,7 @@ void RenderBlock::getFirstLetter(RenderObject*& firstLetter, RenderElement*& fir
             firstLetter = current.firstChild();
     }
     
-    if (!firstLetter)
+    if (!firstLetter || !isRenderBlockFlowOrRenderButton(*firstLetterContainer))
         firstLetterContainer = nullptr;
 }
 
@@ -3305,7 +3304,7 @@ void RenderBlock::updateFirstLetter()
     // be contained within multiple RenderElements.
     getFirstLetter(firstLetterObj, firstLetterContainer);
 
-    if (!firstLetterObj)
+    if (!firstLetterObj || !firstLetterContainer)
         return;
 
     // If the child already has style, then it has already been created, so we just want