From 755a7f108c2d0bec590b95beb579caae9c6fc17c Mon Sep 17 00:00:00 2001 From: Emilio Cobos Alvarez Date: Sat, 2 Sep 2017 09:14:52 +0000 Subject: [PATCH] Merge r221501 - Wrong getComputedStyle behavior for pseudo-elements for layout-dependent properties. https://bugs.webkit.org/show_bug.cgi?id=175936 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patch by Emilio Cobos Álvarez on 2017-09-01 Reviewed by Antti Koivisto. LayoutTests/imported/w3c: * web-platform-tests/cssom/getComputedStyle-pseudo-expected.txt: * web-platform-tests/cssom/getComputedStyle-pseudo.html: Sync test with upstream. Source/WebCore: Before this patch we may wrongly end up using the wrong renderer to resolve this. Tests: imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo.html * css/CSSComputedStyleDeclaration.cpp: (WebCore::ComputedStyleExtractor::styledElement const): (WebCore::ComputedStyleExtractor::styledRenderer const): (WebCore::ComputedStyleExtractor::propertyValue): * css/CSSComputedStyleDeclaration.h: --- LayoutTests/imported/w3c/ChangeLog | 10 +++++++ .../getComputedStyle-pseudo-expected.txt | 2 ++ .../cssom/getComputedStyle-pseudo.html | 27 +++++++++++++++++++ Source/WebCore/ChangeLog | 18 +++++++++++++ .../css/CSSComputedStyleDeclaration.cpp | 19 ++++++++++--- .../WebCore/css/CSSComputedStyleDeclaration.h | 14 +++++++--- 6 files changed, 82 insertions(+), 8 deletions(-) diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog index cfe1020e1ab8..c1da07671066 100644 --- a/LayoutTests/imported/w3c/ChangeLog +++ b/LayoutTests/imported/w3c/ChangeLog @@ -1,3 +1,13 @@ +2017-09-01 Emilio Cobos Álvarez + + Wrong getComputedStyle behavior for pseudo-elements for layout-dependent properties. + https://bugs.webkit.org/show_bug.cgi?id=175936 + + Reviewed by Antti Koivisto. + + * web-platform-tests/cssom/getComputedStyle-pseudo-expected.txt: + * web-platform-tests/cssom/getComputedStyle-pseudo.html: Sync test with upstream. + 2017-08-14 Chris Dumez XHR should only fire an abort event if the cancellation was requested by the client diff --git a/LayoutTests/imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo-expected.txt index 560a26aa0260..520ecd892164 100644 --- a/LayoutTests/imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo-expected.txt +++ b/LayoutTests/imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo-expected.txt @@ -1,4 +1,6 @@ PASS Resolution of width is correct for ::before and ::after pseudo-elements FAIL Resolution of width is correct for ::before and ::after pseudo-elements of display: contents elements assert_equals: expected "50px" but got "auto" +FAIL Resolution of nonexistent pseudo-element styles assert_equals: Nonexistent :before pseudo-element shouldn't claim to have the same style as the originating element expected "static" but got "relative" +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 "" diff --git a/LayoutTests/imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo.html b/LayoutTests/imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo.html index 8efd484fc21d..c8a4506e11c5 100644 --- a/LayoutTests/imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo.html +++ b/LayoutTests/imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo.html @@ -23,9 +23,17 @@ height: 10px; display: block; } +#none { + display: none; +} +#none::before, +#none::after { + content: "Foo"; +}
+
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index f00f3a2b3c47..017ae2abdb34 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,21 @@ +2017-09-01 Emilio Cobos Álvarez + + Wrong getComputedStyle behavior for pseudo-elements for layout-dependent properties. + https://bugs.webkit.org/show_bug.cgi?id=175936 + + Reviewed by Antti Koivisto. + + Before this patch we may wrongly end up using the wrong renderer to resolve + this. + + Tests: imported/w3c/web-platform-tests/cssom/getComputedStyle-pseudo.html + + * css/CSSComputedStyleDeclaration.cpp: + (WebCore::ComputedStyleExtractor::styledElement const): + (WebCore::ComputedStyleExtractor::styledRenderer const): + (WebCore::ComputedStyleExtractor::propertyValue): + * css/CSSComputedStyleDeclaration.h: + 2017-09-01 Simon Fraser transformCanLikelyUseFastPath() can read off the end of a string diff --git a/Source/WebCore/css/CSSComputedStyleDeclaration.cpp b/Source/WebCore/css/CSSComputedStyleDeclaration.cpp index 7ec8eb862410..3489ccccbfb8 100644 --- a/Source/WebCore/css/CSSComputedStyleDeclaration.cpp +++ b/Source/WebCore/css/CSSComputedStyleDeclaration.cpp @@ -2369,7 +2369,7 @@ static bool isLayoutDependent(CSSPropertyID propertyID, const RenderStyle* style } } -Element* ComputedStyleExtractor::styledElement() +Element* ComputedStyleExtractor::styledElement() const { if (!m_element) return nullptr; @@ -2381,6 +2381,17 @@ Element* ComputedStyleExtractor::styledElement() return m_element.get(); } + +RenderElement* ComputedStyleExtractor::styledRenderer() const +{ + auto* element = styledElement(); + if (!element) + return nullptr; + if (m_pseudoElementSpecifier != NOPSEUDO && element == m_element.get()) + return nullptr; + return element->renderer(); +} + static bool isImplicitlyInheritedGridOrFlexProperty(CSSPropertyID propertyID) { // It would be nice if grid and flex worked within normal CSS mechanisms and not invented their own inheritance system. @@ -2658,7 +2669,7 @@ RefPtr ComputedStyleExtractor::propertyValue(CSSPropertyID propertyID, std::unique_ptr ownedStyle; const RenderStyle* style = nullptr; - RenderObject* renderer = nullptr; + RenderElement* renderer = nullptr; bool forceFullLayout = false; if (updateLayout) { Document& document = m_element->document(); @@ -2667,7 +2678,7 @@ RefPtr ComputedStyleExtractor::propertyValue(CSSPropertyID propertyID, // Style update may change styledElement() to PseudoElement or back. styledElement = this->styledElement(); } - renderer = styledElement->renderer(); + renderer = styledRenderer(); if (propertyID == CSSPropertyDisplay && !renderer && is(*styledElement) && !downcast(*styledElement).isValid()) return nullptr; @@ -2687,7 +2698,7 @@ RefPtr ComputedStyleExtractor::propertyValue(CSSPropertyID propertyID, if (!updateLayout || forceFullLayout) { style = computeRenderStyleForProperty(*styledElement, m_pseudoElementSpecifier, propertyID, ownedStyle); - renderer = styledElement->renderer(); + renderer = styledRenderer(); } if (!style) diff --git a/Source/WebCore/css/CSSComputedStyleDeclaration.h b/Source/WebCore/css/CSSComputedStyleDeclaration.h index 2c9e5bc0df2b..13b6d9fdbf21 100644 --- a/Source/WebCore/css/CSSComputedStyleDeclaration.h +++ b/Source/WebCore/css/CSSComputedStyleDeclaration.h @@ -38,7 +38,7 @@ class FilterOperations; class FontSelectionValue; class MutableStyleProperties; class Node; -class RenderObject; +class RenderElement; class RenderStyle; class SVGPaint; class ShadowData; @@ -75,9 +75,15 @@ class ComputedStyleExtractor { static Ref fontStyleFromStyleValue(FontSelectionValue, FontStyleAxis); private: - // The styled element is either the element passed into computedPropertyValue, or the - // PseudoElement for :before and :after if they exist. - Element* styledElement(); + // The styled element is either the element passed into + // computedPropertyValue, or the PseudoElement for :before and :after if + // they exist. + Element* styledElement() const; + + // The renderer we should use for resolving layout-dependent properties. + // Note that it differs from styledElement()->renderer() in the case we have + // no pseudo-element. + RenderElement* styledRenderer() const; RefPtr svgPropertyValue(CSSPropertyID, EUpdateLayout); RefPtr adjustSVGPaintForCurrentColor(SVGPaintType, const String& url, const Color&, const Color& currentColor) const;