- Revision
- 225049
- Author
- [email protected]
- Date
- 2017-11-20 04:32:07 -0800 (Mon, 20 Nov 2017)
Log Message
Incorrect computed style in pseudo-elements with display: contents
https://bugs.webkit.org/show_bug.cgi?id=179812
Reviewed by Antti Koivisto.
LayoutTests/imported/w3c:
* web-platform-tests/cssom/getComputedStyle-pseudo-expected.txt:
* web-platform-tests/cssom/getComputedStyle-pseudo.html:
Extended the testcase to cover this bug.
Source/WebCore:
Bug 178513 gave them an anon renderer as the main renderer. Unfortunately, the
style of that box is exposed via getComputedStyle, so that needs to be handled
somehow to return the correct style.
Tests: imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo.html
* css/CSSComputedStyleDeclaration.cpp:
(WebCore::ComputedStyleExtractor::styledRenderer const):
Don't return a display: contents pseudo anon box.
* dom/Element.cpp:
(WebCore::Element::hasDisplayContents const):
Just look at the rare data style.
(WebCore::Element::existingComputedStyle const):
Look at the rare data first. This also fixes an inefficiency in the
sense that before then we'd compare the anon box style to the
pseudo-element style during style update, that may be more changes
than needed.
* style/RenderTreeUpdaterGeneratedContent.cpp:
(WebCore::RenderTreeUpdater::GeneratedContent::updatePseudoElement):
* style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::resolvePseudoStyle):
Move the anon box creation to render tree update instead of style
resolution.
Modified Paths
Diff
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (225048 => 225049)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2017-11-20 11:22:30 UTC (rev 225048)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2017-11-20 12:32:07 UTC (rev 225049)
@@ -1,3 +1,14 @@
+2017-11-20 Emilio Cobos Álvarez <[email protected]>
+
+ Incorrect computed style in pseudo-elements with display: contents
+ https://bugs.webkit.org/show_bug.cgi?id=179812
+
+ Reviewed by Antti Koivisto.
+
+ * web-platform-tests/cssom/getComputedStyle-pseudo-expected.txt:
+ * web-platform-tests/cssom/getComputedStyle-pseudo.html:
+ Extended the testcase to cover this bug.
+
2017-11-17 Youenn Fablet <[email protected]>
ServiceWorker intercepted FetchRequest should have their referrer set appropriately.
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo-expected.txt (225048 => 225049)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo-expected.txt 2017-11-20 11:22:30 UTC (rev 225048)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo-expected.txt 2017-11-20 12:32:07 UTC (rev 225049)
@@ -5,4 +5,5 @@
FAIL Resolution of pseudo-element styles in display: none elements assert_equals: Pseudo-styles of display: none elements should be correct expected "\"Foo\"" but got "Foo"
PASS Item-based blockification of pseudo-elements
FAIL Item-based blockification of nonexistent pseudo-elements assert_equals: Pseudo-styles of display: flex elements should get blockified expected "block" but got "inline"
+PASS display: contents on pseudo-elements
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo.html (225048 => 225049)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo.html 2017-11-20 11:22:30 UTC (rev 225048)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo.html 2017-11-20 12:32:07 UTC (rev 225049)
@@ -38,6 +38,12 @@
#flex-no-pseudo {
display: flex;
}
+#contents-pseudos::before,
+#contents-pseudos::after {
+ display: contents;
+ content: "foo";
+ position: absolute;
+}
</style>
<div id="test">
<div id="contents"></div>
@@ -44,6 +50,7 @@
<div id="none"></div>
<div id="flex"></div>
<div id="flex-no-pseudo"></div>
+ <div id="contents-pseudos"></div>
</div>
<script>
test(function() {
@@ -91,4 +98,15 @@
"Pseudo-styles of display: flex elements should get blockified");
});
}, "Item-based blockification of nonexistent pseudo-elements");
+test(function() {
+ var contentsPseudos = document.getElementById('contents-pseudos');
+ [":before", ":after"].forEach(function(pseudo) {
+ assert_equals(getComputedStyle(contentsPseudos, pseudo).display, "contents",
+ "display: contents in " + pseudo + " should get reflected on CSSOM");
+ assert_equals(getComputedStyle(contentsPseudos, pseudo).width, "auto",
+ pseudo + " with display: contents should have no box");
+ assert_equals(getComputedStyle(contentsPseudos, pseudo).position, "absolute",
+ "display: contents in " + pseudo + " should reflect other non-inherited properties in CSSOM");
+ });
+}, "display: contents on pseudo-elements");
</script>
Modified: trunk/Source/WebCore/ChangeLog (225048 => 225049)
--- trunk/Source/WebCore/ChangeLog 2017-11-20 11:22:30 UTC (rev 225048)
+++ trunk/Source/WebCore/ChangeLog 2017-11-20 12:32:07 UTC (rev 225049)
@@ -1,3 +1,34 @@
+2017-11-20 Emilio Cobos Álvarez <[email protected]>
+
+ Incorrect computed style in pseudo-elements with display: contents
+ https://bugs.webkit.org/show_bug.cgi?id=179812
+
+ Reviewed by Antti Koivisto.
+
+ Bug 178513 gave them an anon renderer as the main renderer. Unfortunately, the
+ style of that box is exposed via getComputedStyle, so that needs to be handled
+ somehow to return the correct style.
+
+ Tests: imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo.html
+
+ * css/CSSComputedStyleDeclaration.cpp:
+ (WebCore::ComputedStyleExtractor::styledRenderer const):
+ Don't return a display: contents pseudo anon box.
+ * dom/Element.cpp:
+ (WebCore::Element::hasDisplayContents const):
+ Just look at the rare data style.
+ (WebCore::Element::existingComputedStyle const):
+ Look at the rare data first. This also fixes an inefficiency in the
+ sense that before then we'd compare the anon box style to the
+ pseudo-element style during style update, that may be more changes
+ than needed.
+ * style/RenderTreeUpdaterGeneratedContent.cpp:
+ (WebCore::RenderTreeUpdater::GeneratedContent::updatePseudoElement):
+ * style/StyleTreeResolver.cpp:
+ (WebCore::Style::TreeResolver::resolvePseudoStyle):
+ Move the anon box creation to render tree update instead of style
+ resolution.
+
2017-11-19 Tim Horton <[email protected]>
Remove unused TOUCH_ICON_LOADING feature flag
Modified: trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp (225048 => 225049)
--- trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp 2017-11-20 11:22:30 UTC (rev 225048)
+++ trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp 2017-11-20 12:32:07 UTC (rev 225049)
@@ -2360,6 +2360,8 @@
return nullptr;
if (m_pseudoElementSpecifier != NOPSEUDO && element == m_element.get())
return nullptr;
+ if (element->hasDisplayContents())
+ return nullptr;
return element->renderer();
}
Modified: trunk/Source/WebCore/dom/Element.cpp (225048 => 225049)
--- trunk/Source/WebCore/dom/Element.cpp 2017-11-20 11:22:30 UTC (rev 225048)
+++ trunk/Source/WebCore/dom/Element.cpp 2017-11-20 12:32:07 UTC (rev 225049)
@@ -1514,8 +1514,9 @@
bool Element::hasDisplayContents() const
{
- if (renderer() || !hasRareData())
+ if (!hasRareData())
return false;
+
const RenderStyle* style = elementRareData()->computedStyle();
return style && style->display() == CONTENTS;
}
@@ -1523,7 +1524,7 @@
void Element::storeDisplayContentsStyle(std::unique_ptr<RenderStyle> style)
{
ASSERT(style && style->display() == CONTENTS);
- ASSERT(!renderer());
+ ASSERT(!renderer() || isPseudoElement());
ensureElementRareData().setComputedStyle(WTFMove(style));
}
@@ -2673,13 +2674,12 @@
const RenderStyle* Element::existingComputedStyle() const
{
- if (auto* renderTreeStyle = renderStyle())
- return renderTreeStyle;
+ if (hasRareData()) {
+ if (auto* style = elementRareData()->computedStyle())
+ return style;
+ }
- if (hasRareData())
- return elementRareData()->computedStyle();
-
- return nullptr;
+ return renderStyle();
}
const RenderStyle& Element::resolveComputedStyle()
Modified: trunk/Source/WebCore/style/RenderTreeUpdaterGeneratedContent.cpp (225048 => 225049)
--- trunk/Source/WebCore/style/RenderTreeUpdaterGeneratedContent.cpp 2017-11-20 11:22:30 UTC (rev 225048)
+++ trunk/Source/WebCore/style/RenderTreeUpdaterGeneratedContent.cpp 2017-11-20 12:32:07 UTC (rev 225049)
@@ -123,8 +123,22 @@
current.setAfterPseudoElement(newPseudoElement.releaseNonNull());
}
- m_updater.updateElementRenderer(*pseudoElement, *update);
+ if (update->style->display() == CONTENTS) {
+ // For display:contents we create an inline wrapper that inherits its
+ // style from the display:contents style.
+ auto contentsStyle = RenderStyle::createPtr();
+ contentsStyle->setStyleType(pseudoId);
+ contentsStyle->inheritFrom(*update->style);
+ contentsStyle->copyContentFrom(*update->style);
+ Style::ElementUpdate contentsUpdate { WTFMove(contentsStyle), update->change, update->recompositeLayer };
+ m_updater.updateElementRenderer(*pseudoElement, contentsUpdate);
+ pseudoElement->storeDisplayContentsStyle(RenderStyle::clonePtr(*update->style));
+ } else {
+ m_updater.updateElementRenderer(*pseudoElement, *update);
+ ASSERT(!pseudoElement->hasDisplayContents());
+ }
+
auto* pseudoElementRenderer = pseudoElement->renderer();
if (!pseudoElementRenderer)
return;
Modified: trunk/Source/WebCore/style/StyleTreeResolver.cpp (225048 => 225049)
--- trunk/Source/WebCore/style/StyleTreeResolver.cpp 2017-11-20 11:22:30 UTC (rev 225048)
+++ trunk/Source/WebCore/style/StyleTreeResolver.cpp 2017-11-20 12:32:07 UTC (rev 225049)
@@ -247,15 +247,6 @@
element.setAfterPseudoElement(WTFMove(newPseudoElement));
}
- if (pseudoStyle->display() == CONTENTS) {
- // For display:contents we create an inline wrapper that inherits its style from the display:contents style.
- auto contentsStyle = RenderStyle::createPtr();
- contentsStyle->setStyleType(pseudoId);
- contentsStyle->inheritFrom(*pseudoStyle);
- contentsStyle->copyContentFrom(*pseudoStyle);
- pseudoStyle = WTFMove(contentsStyle);
- }
-
return createAnimatedElementUpdate(WTFMove(pseudoStyle), *pseudoElement, elementUpdate.change);
}
