From a566a3386dce5eaff1cd58c0b0081af44abdea7b Mon Sep 17 00:00:00 2001 From: "rego@igalia.com" Date: Tue, 9 Sep 2014 21:58:11 +0000 Subject: [PATCH] [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. git-svn-id: https://svn.webkit.org/repository/webkit/trunk@173444 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- LayoutTests/ChangeLog | 15 ++++++ .../flexbox-ignore-container-firstLetter.html | 2 +- ...grid-container-ignore-first-letter-expected.txt | 22 ++++++++ .../grid-container-ignore-first-letter.html | 59 ++++++++++++++++++++++ .../grid-item-first-letter-valid-expected.txt | 8 +++ .../grid-item-first-letter-valid.html | 23 +++++++++ Source/WebCore/ChangeLog | 36 +++++++++++++ Source/WebCore/rendering/RenderBlock.cpp | 33 ++++++------ 8 files changed, 180 insertions(+), 18 deletions(-) create mode 100644 LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter-expected.txt create mode 100644 LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter.html create mode 100644 LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid-expected.txt create mode 100644 LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid.html diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index d724bfc..8e1ccf4 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,18 @@ +2014-09-09 Manuel Rego Casasnovas + + [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 Add support for :read-write/:read-only matching editable content diff --git a/LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter.html b/LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter.html index f30b9b4..48aa5bb 100644 --- a/LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter.html +++ b/LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter.html @@ -2,8 +2,8 @@ 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 index 0000000..8f9afac --- /dev/null +++ b/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter-expected.txt @@ -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 index 0000000..15b2f43 --- /dev/null +++ b/LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter.html @@ -0,0 +1,59 @@ + + + + + + + +

This test grid item should ignore grid container's first-letter pseudo-element.

+ +
+
The first item.
+
The second item.
+
+ +
+
The first item.
+
The second item.
+
+ +
+ Anonymous item. +
+ +
+ Anonymous item. +
+ +
+
+
The first item.
+
The second item.
+
+
+ +
+
+
The first item.
+
The second item.
+
+
+ +
+
+ Anonymous item. +
+
+ +
+
+ Anonymous item. +
+
+ + + 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 index 0000000..6e8bae5 --- /dev/null +++ b/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid-expected.txt @@ -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 index 0000000..b980093 --- /dev/null +++ b/LayoutTests/fast/css-grid-layout/grid-item-first-letter-valid.html @@ -0,0 +1,23 @@ + + + + + + + +

This test grid item's first-letter pseudo-element should be valid.

+ +
+
The first item.
+
The second item.
+
+ +
+
The first item.
+
The second item.
+
+ + + diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index eb6a2ce..c0138ae 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,39 @@ +2014-09-09 Manuel Rego Casasnovas + + [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 Add support for :read-write/:read-only matching editable content diff --git a/Source/WebCore/rendering/RenderBlock.cpp b/Source/WebCore/rendering/RenderBlock.cpp index 6bbe4be..1f31242 100644 --- a/Source/WebCore/rendering/RenderBlock.cpp +++ b/Source/WebCore/rendering/RenderBlock.cpp @@ -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(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 -- 1.8.3.1