- Revision
- 173444
- Author
- [email protected]
- Date
- 2014-09-09 14:58:11 -0700 (Tue, 09 Sep 2014)
Log Message
[CSS Grid Layout] Ignore ::first-letter pseudo-element
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (173443 => 173444)
--- trunk/LayoutTests/ChangeLog 2014-09-09 21:49:21 UTC (rev 173443)
+++ trunk/LayoutTests/ChangeLog 2014-09-09 21:58:11 UTC (rev 173444)
@@ -1,3 +1,18 @@
+2014-09-09 Manuel Rego Casasnovas <[email protected]>
+
+ [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 <[email protected]>
Add support for :read-write/:read-only matching editable content
Modified: trunk/LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter.html (173443 => 173444)
--- trunk/LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter.html 2014-09-09 21:49:21 UTC (rev 173443)
+++ trunk/LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter.html 2014-09-09 21:58:11 UTC (rev 173444)
@@ -2,8 +2,8 @@
<html>
<link href="" rel="stylesheet">
<style>
- .container { display: -webkit-flex }
.container::first-letter { line-height: 100px; }
+ .flexbox::first-letter { line-height: 200px; }
p { line-height: 20px; }
</style>
Added: trunk/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter-expected.txt (0 => 173444)
--- trunk/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter-expected.txt 2014-09-09 21:58:11 UTC (rev 173444)
@@ -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
Added: trunk/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter.html (0 => 173444)
--- trunk/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter.html (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter.html 2014-09-09 21:58:11 UTC (rev 173444)
@@ -0,0 +1,59 @@
+<!DOCTYPE html>
+<html>
+<link href="" 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=""
+<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>
Added: trunk/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid-expected.txt (0 => 173444)
--- trunk/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid-expected.txt 2014-09-09 21:58:11 UTC (rev 173444)
@@ -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
Added: trunk/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid.html (0 => 173444)
--- trunk/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid.html (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid.html 2014-09-09 21:58:11 UTC (rev 173444)
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<link href="" rel="stylesheet">
+<style>
+ .item::first-letter { line-height: 100px; }
+</style>
+
+<script src=""
+<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>
Modified: trunk/Source/WebCore/ChangeLog (173443 => 173444)
--- trunk/Source/WebCore/ChangeLog 2014-09-09 21:49:21 UTC (rev 173443)
+++ trunk/Source/WebCore/ChangeLog 2014-09-09 21:58:11 UTC (rev 173444)
@@ -1,3 +1,39 @@
+2014-09-09 Manuel Rego Casasnovas <[email protected]>
+
+ [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 <[email protected]>
Add support for :read-write/:read-only matching editable content
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (173443 => 173444)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2014-09-09 21:49:21 UTC (rev 173443)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2014-09-09 21:58:11 UTC (rev 173444)
@@ -3019,6 +3019,17 @@
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 @@
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 @@
{
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 @@
firstLetter = current.firstChild();
}
- if (!firstLetter)
+ if (!firstLetter || !isRenderBlockFlowOrRenderButton(*firstLetterContainer))
firstLetterContainer = nullptr;
}
@@ -3305,7 +3304,7 @@
// 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
