<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[173444] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/173444">173444</a></dd>
<dt>Author</dt> <dd>rego@igalia.com</dd>
<dt>Date</dt> <dd>2014-09-09 14:58:11 -0700 (Tue, 09 Sep 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>[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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestscss3flexboxflexboxignorecontainerfirstLetterhtml">trunk/LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter.html</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderBlockcpp">trunk/Source/WebCore/rendering/RenderBlock.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastcssgridlayoutgridcontainerignorefirstletterexpectedtxt">trunk/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastcssgridlayoutgridcontainerignorefirstletterhtml">trunk/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter.html</a></li>
<li><a href="#trunkLayoutTestsfastcssgridlayoutgriditemfirstlettervalidexpectedtxt">trunk/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastcssgridlayoutgriditemfirstlettervalidhtml">trunk/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (173443 => 173444)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2014-09-09 21:49:21 UTC (rev 173443)
+++ trunk/LayoutTests/ChangeLog 2014-09-09 21:58:11 UTC (rev 173444)
</span><span class="lines">@@ -1,3 +1,18 @@
</span><ins>+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.
+
</ins><span class="cx"> 2014-09-09 Benjamin Poulain <benjamin@webkit.org>
</span><span class="cx">
</span><span class="cx"> Add support for :read-write/:read-only matching editable content
</span></span></pre></div>
<a id="trunkLayoutTestscss3flexboxflexboxignorecontainerfirstLetterhtml"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter.html (173443 => 173444)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -2,8 +2,8 @@
</span><span class="cx"> <html>
</span><span class="cx"> <link href="resources/flexbox.css" rel="stylesheet">
</span><span class="cx"> <style>
</span><del>- .container { display: -webkit-flex }
</del><span class="cx"> .container::first-letter { line-height: 100px; }
</span><ins>+ .flexbox::first-letter { line-height: 200px; }
</ins><span class="cx"> p { line-height: 20px; }
</span><span class="cx"> </style>
</span><span class="cx">
</span></span></pre></div>
<a id="trunkLayoutTestsfastcssgridlayoutgridcontainerignorefirstletterexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter-expected.txt (0 => 173444)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,22 @@
</span><ins>+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
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssgridlayoutgridcontainerignorefirstletterhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter.html (0 => 173444)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,59 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssgridlayoutgriditemfirstlettervalidexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid-expected.txt (0 => 173444)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,8 @@
</span><ins>+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
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssgridlayoutgriditemfirstlettervalidhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid.html (0 => 173444)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,23 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (173443 => 173444)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,39 @@
</span><ins>+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).
+
</ins><span class="cx"> 2014-09-09 Benjamin Poulain <benjamin@webkit.org>
</span><span class="cx">
</span><span class="cx"> Add support for :read-write/:read-only matching editable content
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderBlockcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (173443 => 173444)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -3019,6 +3019,17 @@
</span><span class="cx"> return -1;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+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();
+}
+
</ins><span class="cx"> RenderBlock* RenderBlock::firstLineBlock() const
</span><span class="cx"> {
</span><span class="cx"> RenderBlock* firstLineBlock = const_cast<RenderBlock*>(this);
</span><span class="lines">@@ -3028,14 +3039,8 @@
</span><span class="cx"> if (hasPseudo)
</span><span class="cx"> break;
</span><span class="cx"> RenderElement* parentBlock = firstLineBlock->parent();
</span><del>- // 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.
</del><span class="cx"> if (firstLineBlock->isReplaced() || firstLineBlock->isFloating()
</span><del>- || !parentBlock || parentBlock->firstChild() != firstLineBlock || (!parentBlock->isRenderBlockFlow() && !parentBlock->isRenderButton()))
</del><ins>+ || !parentBlock || parentBlock->firstChild() != firstLineBlock || !isRenderBlockFlowOrRenderButton(*parentBlock))
</ins><span class="cx"> break;
</span><span class="cx"> firstLineBlock = toRenderBlock(parentBlock);
</span><span class="cx"> }
</span><span class="lines">@@ -3111,21 +3116,15 @@
</span><span class="cx"> {
</span><span class="cx"> RenderBlock* firstLetterBlock = start;
</span><span class="cx"> while (true) {
</span><del>- // 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.
</del><span class="cx"> bool canHaveFirstLetterRenderer = firstLetterBlock->style().hasPseudoStyle(FIRST_LETTER)
</span><span class="cx"> && firstLetterBlock->canHaveGeneratedChildren()
</span><del>- && (!firstLetterBlock->isFlexibleBox() || firstLetterBlock->isRenderButton());
</del><ins>+ && isRenderBlockFlowOrRenderButton(*firstLetterBlock);
</ins><span class="cx"> if (canHaveFirstLetterRenderer)
</span><span class="cx"> return firstLetterBlock;
</span><span class="cx">
</span><span class="cx"> RenderElement* parentBlock = firstLetterBlock->parent();
</span><span class="cx"> if (firstLetterBlock->isReplaced() || !parentBlock || parentBlock->firstChild() != firstLetterBlock
</span><del>- || (!parentBlock->isRenderBlockFlow() && !parentBlock->isRenderButton()))
</del><ins>+ || !isRenderBlockFlowOrRenderButton(*parentBlock))
</ins><span class="cx"> return 0;
</span><span class="cx"> firstLetterBlock = toRenderBlock(parentBlock);
</span><span class="cx"> }
</span><span class="lines">@@ -3293,7 +3292,7 @@
</span><span class="cx"> firstLetter = current.firstChild();
</span><span class="cx"> }
</span><span class="cx">
</span><del>- if (!firstLetter)
</del><ins>+ if (!firstLetter || !isRenderBlockFlowOrRenderButton(*firstLetterContainer))
</ins><span class="cx"> firstLetterContainer = nullptr;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -3305,7 +3304,7 @@
</span><span class="cx"> // be contained within multiple RenderElements.
</span><span class="cx"> getFirstLetter(firstLetterObj, firstLetterContainer);
</span><span class="cx">
</span><del>- if (!firstLetterObj)
</del><ins>+ if (!firstLetterObj || !firstLetterContainer)
</ins><span class="cx"> return;
</span><span class="cx">
</span><span class="cx"> // If the child already has style, then it has already been created, so we just want
</span></span></pre>
</div>
</div>
</body>
</html>