Skip to content

GitLab

T
tangled
Commit dd8c2a6b authored by rego@igalia.com's avatar rego@igalia.com
Browse files
Options

[CSS Grid Layout] Ignore ::first-letter pseudo-element 

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

Modified RenderBlock::findFirstLetterBlock() to use isRenderBlockFlow()
instead of isFlexibleBox(). This change has been already made before in
several parts of the code, but there was one missing case here.

Fix issue in RenderBlock::updateFirstLetter() 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.
Add the proper check to avoid this.

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

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

BUG=395788

Review URL: https://codereview.chromium.org/440233002

git-svn-id: svn://svn.chromium.org/blink/trunk@180043 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 881aa465
Hide whitespace changes
Inline Side-by-side
Showing with 128 additions and 17 deletions
third_party/WebKit/LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter.html
View file @ dd8c2a6b
...@@ -2,8 +2,8 @@ ...@@ -2,8 +2,8 @@
<html> <html>
<link href="resources/flexbox.css" rel="stylesheet"> <link href="resources/flexbox.css" rel="stylesheet">
<style> <style>
.container { display: -webkit-flex }
.container::first-letter { line-height: 100px; } .container::first-letter { line-height: 100px; }
.flexbox::first-letter { line-height: 200px; }
p { line-height: 20px; } p { line-height: 20px; }
</style> </style>
......
third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter-expected.txt 0 → 100644
View file @ dd8c2a6b
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
third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter.html 0 → 100644
View file @ dd8c2a6b
<!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>
third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid-expected.txt 0 → 100644
View file @ dd8c2a6b
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
third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid.html 0 → 100644
View file @ dd8c2a6b
<!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>
third_party/WebKit/Source/core/rendering/RenderBlock.cpp
View file @ dd8c2a6b
...@@ -3929,6 +3929,17 @@ int RenderBlock::lastLineBoxBaseline(LineDirectionMode lineDirection) const ...@@ -3929,6 +3929,17 @@ int RenderBlock::lastLineBoxBaseline(LineDirectionMode lineDirection) const
return -1; return -1;
} }
static inline bool isRenderBlockFlowOrRenderButton(RenderObject* renderObject)
{
// 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 grid 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 renderObject->isRenderBlockFlow() || renderObject->isRenderButton();
}
RenderBlock* RenderBlock::firstLineBlock() const RenderBlock* RenderBlock::firstLineBlock() const
{ {
RenderBlock* firstLineBlock = const_cast<RenderBlock*>(this); RenderBlock* firstLineBlock = const_cast<RenderBlock*>(this);
...@@ -3938,15 +3949,9 @@ RenderBlock* RenderBlock::firstLineBlock() const ...@@ -3938,15 +3949,9 @@ RenderBlock* RenderBlock::firstLineBlock() const
if (hasPseudo) if (hasPseudo)
break; break;
RenderObject* parentBlock = firstLineBlock->parent(); RenderObject* 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() if (firstLineBlock->isReplaced() || firstLineBlock->isFloating()
|| !parentBlock || !parentBlock
|| (!parentBlock->isRenderBlockFlow() && !parentBlock->isRenderButton())) || !isRenderBlockFlowOrRenderButton(parentBlock))
break; break;
ASSERT_WITH_SECURITY_IMPLICATION(parentBlock->isRenderBlock()); ASSERT_WITH_SECURITY_IMPLICATION(parentBlock->isRenderBlock());
if (toRenderBlock(parentBlock)->firstChild() != firstLineBlock) if (toRenderBlock(parentBlock)->firstChild() != firstLineBlock)
...@@ -3992,21 +3997,15 @@ static inline RenderObject* findFirstLetterBlock(RenderBlock* start) ...@@ -3992,21 +3997,15 @@ static inline RenderObject* findFirstLetterBlock(RenderBlock* start)
{ {
RenderObject* firstLetterBlock = start; RenderObject* firstLetterBlock = start;
while (true) { 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) bool canHaveFirstLetterRenderer = firstLetterBlock->style()->hasPseudoStyle(FIRST_LETTER)
&& firstLetterBlock->canHaveGeneratedChildren() && firstLetterBlock->canHaveGeneratedChildren()
&& (!firstLetterBlock->isFlexibleBox() || firstLetterBlock->isRenderButton()); && isRenderBlockFlowOrRenderButton(firstLetterBlock);
if (canHaveFirstLetterRenderer) if (canHaveFirstLetterRenderer)
return firstLetterBlock; return firstLetterBlock;
RenderObject* parentBlock = firstLetterBlock->parent(); RenderObject* parentBlock = firstLetterBlock->parent();
if (firstLetterBlock->isReplaced() || !parentBlock if (firstLetterBlock->isReplaced() || !parentBlock
|| (!parentBlock->isRenderBlockFlow() && !parentBlock->isRenderButton())) { || !isRenderBlockFlowOrRenderButton(parentBlock)) {
return 0; return 0;
} }
ASSERT(parentBlock->isRenderBlock()); ASSERT(parentBlock->isRenderBlock());
...@@ -4189,7 +4188,7 @@ void RenderBlock::updateFirstLetter() ...@@ -4189,7 +4188,7 @@ void RenderBlock::updateFirstLetter()
} }
} }
if (!currChild) if (!currChild || !isRenderBlockFlowOrRenderButton(firstLetterBlock))
return; return;
// If the child already has style, then it has already been created, so we just want // If the child already has style, then it has already been created, so we just want
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment