Diff
Modified: trunk/LayoutTests/ChangeLog (191261 => 191262)
--- trunk/LayoutTests/ChangeLog 2015-10-18 17:19:40 UTC (rev 191261)
+++ trunk/LayoutTests/ChangeLog 2015-10-18 22:15:18 UTC (rev 191262)
@@ -1,3 +1,23 @@
+2015-10-18 Antti Koivisto <an...@apple.com>
+
+ Computed style should work correctly with slotted elements that have display:none
+ https://bugs.webkit.org/show_bug.cgi?id=150237
+
+ Reviewed by Andreas Kling.
+
+ * editing/style/apply-style-atomic-expected.txt:
+
+ Rebase.
+
+ * fast/css/getComputedStyle/getComputedStyle-with-pseudo-element-expected.txt:
+ * fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html:
+
+ We now also compute style of display:none pseudo elements correctly.
+ This is a progression and matches other browsers.
+
+ * fast/shadow-dom/computed-style-display-none-expected.txt: Added.
+ * fast/shadow-dom/computed-style-display-none.html: Added.
+
2015-10-18 Gyuyoung Kim <gyuyoung....@webkit.org>
Unreviewed EFL gardening. Mark css variables tests to pass
Modified: trunk/LayoutTests/editing/style/apply-style-atomic-expected.txt (191261 => 191262)
--- trunk/LayoutTests/editing/style/apply-style-atomic-expected.txt 2015-10-18 17:19:40 UTC (rev 191261)
+++ trunk/LayoutTests/editing/style/apply-style-atomic-expected.txt 2015-10-18 22:15:18 UTC (rev 191262)
@@ -2,19 +2,18 @@
| <a>
| href=""
| "<#selection-anchor>1"
-| <progress>
-| <a>
-| style=""
-| "2"
-| <shadow:root>
+| <progress>
+| <a>
+| style=""
+| "2"
+| <shadow:root>
+| <div>
+| pseudo="-webkit-progress-inner-element"
+| shadow:pseudoId="-webkit-progress-inner-element"
| <div>
-| pseudo="-webkit-progress-inner-element"
-| shadow:pseudoId="-webkit-progress-inner-element"
+| pseudo="-webkit-progress-bar"
+| shadow:pseudoId="-webkit-progress-bar"
| <div>
-| pseudo="-webkit-progress-bar"
-| shadow:pseudoId="-webkit-progress-bar"
-| <div>
-| pseudo="-webkit-progress-value"
-| style="width: -100%;"
-| shadow:pseudoId="-webkit-progress-value"
-| <#selection-focus>
+| pseudo="-webkit-progress-value"
+| style="width: -100%;"
+| shadow:pseudoId="-webkit-progress-value"
Modified: trunk/LayoutTests/fast/css/getComputedStyle/getComputedStyle-with-pseudo-element-expected.txt (191261 => 191262)
--- trunk/LayoutTests/fast/css/getComputedStyle/getComputedStyle-with-pseudo-element-expected.txt 2015-10-18 17:19:40 UTC (rev 191261)
+++ trunk/LayoutTests/fast/css/getComputedStyle/getComputedStyle-with-pseudo-element-expected.txt 2015-10-18 22:15:18 UTC (rev 191262)
@@ -51,10 +51,10 @@
PASS Expected '0px' for padding in the computed style for element with id testBeforeAfterInline and pseudo-element :before and got '0px'
PASS Expected '0px' for margin in the computed style for element with id testBeforeAfterInline and pseudo-element :after and got '0px'
PASS Expected '10px 20px 30px 40px' for padding in the computed style for element with id testBeforeAfterInline and pseudo-element :after and got '10px 20px 30px 40px'
-PASS Expected '' for width in the computed style for element with id testBeforeAfterDisplayNone and pseudo-element :after and got ''
-PASS Expected '' for height in the computed style for element with id testBeforeAfterDisplayNone and pseudo-element :after and got ''
-PASS Expected '' for width in the computed style for element with id testBeforeAfterDisplayNone and pseudo-element :before and got ''
-PASS Expected '' for height in the computed style for element with id testBeforeAfterDisplayNone and pseudo-element :before and got ''
+PASS Expected '100px' for width in the computed style for element with id testBeforeAfterDisplayNone and pseudo-element :after and got '100px'
+PASS Expected '100px' for height in the computed style for element with id testBeforeAfterDisplayNone and pseudo-element :after and got '100px'
+PASS Expected '100px' for width in the computed style for element with id testBeforeAfterDisplayNone and pseudo-element :before and got '100px'
+PASS Expected '100px' for height in the computed style for element with id testBeforeAfterDisplayNone and pseudo-element :before and got '100px'
PASS Expected 'rgb(165, 42, 42)' for color in the computed style for element with id testNoPseudoElement and pseudo-element null and got 'rgb(165, 42, 42)'
PASS Expected 'rgb(165, 42, 42)' for color in the computed style for element with id testNoPseudoElement and pseudo-element :first-line and got 'rgb(165, 42, 42)'
PASS Expected 'rgb(165, 42, 42)' for color in the computed style for element with id testNoPseudoElement and pseudo-element :first-letter and got 'rgb(165, 42, 42)'
Modified: trunk/LayoutTests/fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html (191261 => 191262)
--- trunk/LayoutTests/fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html 2015-10-18 17:19:40 UTC (rev 191261)
+++ trunk/LayoutTests/fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html 2015-10-18 22:15:18 UTC (rev 191262)
@@ -168,10 +168,10 @@
{ 'elementId' : 'testBeforeAfterInline', 'pseudoElement' : ':before', 'property' : 'padding', 'expectedValue' : '0px' },
{ 'elementId' : 'testBeforeAfterInline', 'pseudoElement' : ':after', 'property' : 'margin', 'expectedValue' : '0px' },
{ 'elementId' : 'testBeforeAfterInline', 'pseudoElement' : ':after', 'property' : 'padding', 'expectedValue' : '10px 20px 30px 40px' },
- { 'elementId' : 'testBeforeAfterDisplayNone', 'pseudoElement' : ':after', 'property' : 'width', 'expectedValue' : '' },
- { 'elementId' : 'testBeforeAfterDisplayNone', 'pseudoElement' : ':after', 'property' : 'height', 'expectedValue' : '' },
- { 'elementId' : 'testBeforeAfterDisplayNone', 'pseudoElement' : ':before', 'property' : 'width', 'expectedValue' : '' },
- { 'elementId' : 'testBeforeAfterDisplayNone', 'pseudoElement' : ':before', 'property' : 'height', 'expectedValue' : '' },
+ { 'elementId' : 'testBeforeAfterDisplayNone', 'pseudoElement' : ':after', 'property' : 'width', 'expectedValue' : '100px' },
+ { 'elementId' : 'testBeforeAfterDisplayNone', 'pseudoElement' : ':after', 'property' : 'height', 'expectedValue' : '100px' },
+ { 'elementId' : 'testBeforeAfterDisplayNone', 'pseudoElement' : ':before', 'property' : 'width', 'expectedValue' : '100px' },
+ { 'elementId' : 'testBeforeAfterDisplayNone', 'pseudoElement' : ':before', 'property' : 'height', 'expectedValue' : '100px' },
{ 'elementId' : 'testNoPseudoElement', 'pseudoElement' : null, 'property' : 'color', 'expectedValue' : 'rgb(165, 42, 42)' },
{ 'elementId' : 'testNoPseudoElement', 'pseudoElement' : ':first-line', 'property' : 'color', 'expectedValue' : 'rgb(165, 42, 42)' },
{ 'elementId' : 'testNoPseudoElement', 'pseudoElement' : ':first-letter', 'property' : 'color', 'expectedValue' : 'rgb(165, 42, 42)' },
Added: trunk/LayoutTests/fast/shadow-dom/computed-style-display-none-expected.txt (0 => 191262)
--- trunk/LayoutTests/fast/shadow-dom/computed-style-display-none-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/shadow-dom/computed-style-display-none-expected.txt 2015-10-18 22:15:18 UTC (rev 191262)
@@ -0,0 +1,10 @@
+Ensure tha slotted elements with display:none inherit their style via shadow tree
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS getComputedStyle(hostChild).color is "rgb(0, 128, 0)"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/shadow-dom/computed-style-display-none.html (0 => 191262)
--- trunk/LayoutTests/fast/shadow-dom/computed-style-display-none.html (rev 0)
+++ trunk/LayoutTests/fast/shadow-dom/computed-style-display-none.html 2015-10-18 22:15:18 UTC (rev 191262)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src=""
+</head>
+<style>
+my-host { display: none; color: red; }
+</style>
+<body>
+<my-host><my-host-child></my-host-child></my-host>
+<script>
+description("Ensure tha slotted elements with display:none inherit their style via shadow tree");
+
+var host = document.querySelector("my-host");
+var shadowRoot = host.attachShadow({ mode: "closed"});
+shadowRoot.innerHTML = "<style>div { color: green }</style><div><slot></slot></div>";
+
+hostChild = document.querySelector("my-host-child");
+shouldBeEqualToString("getComputedStyle(hostChild).color", "rgb(0, 128, 0)");
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (191261 => 191262)
--- trunk/Source/WebCore/ChangeLog 2015-10-18 17:19:40 UTC (rev 191261)
+++ trunk/Source/WebCore/ChangeLog 2015-10-18 22:15:18 UTC (rev 191262)
@@ -1,3 +1,47 @@
+2015-10-18 Antti Koivisto <an...@apple.com>
+
+ Computed style should work correctly with slotted elements that have display:none
+ https://bugs.webkit.org/show_bug.cgi?id=150237
+
+ Reviewed by Andreas Kling..
+
+ If an element has display:none we don't normally retain or even compute its style (as it is not rendered).
+ If getComputedStyle is invoked for such element we resolve the style (along with any ancestors) and cache
+ it separately to rare data. This path needs to work with slotted elements in shadow trees.
+
+ This patch also make computedStyle() iterative rather than recursive.
+
+ Test: fast/shadow-dom/computed-style-display-none.html
+
+ * dom/Document.cpp:
+ (WebCore::Document::styleForElementIgnoringPendingStylesheets):
+
+ Pass in the parent style instead of invoking computedStyle() recursively.
+
+ * dom/Document.h:
+ * dom/Element.cpp:
+ (WebCore::beforeOrAfterPseudoElement):
+ (WebCore::Element::existingComputedStyle):
+ (WebCore::Element::resolveComputedStyle):
+
+ Iterative resolve function that uses composed tree iterator.
+
+ (WebCore::Element::computedStyle):
+
+ Factor into helpers.
+
+ * dom/Element.h:
+ * dom/Node.cpp:
+ (WebCore::Node::computedStyle):
+
+ Use the composed tree iterator.
+
+ * html/HTMLSelectElement.cpp:
+ (WebCore::HTMLSelectElement::selectOption):
+
+ Call updateValidity() before calling renderer->updateFromElement(). Calling updateFromElement()
+ may end up in Element::computedStyle() which can asserts if validity is not up to date.
+
2015-10-18 Myles C. Maxfield <mmaxfi...@apple.com>
Stop honoring the user default "WebKitKerningAndLigaturesEnabledByDefault"
Modified: trunk/Source/WebCore/dom/Document.cpp (191261 => 191262)
--- trunk/Source/WebCore/dom/Document.cpp 2015-10-18 17:19:40 UTC (rev 191261)
+++ trunk/Source/WebCore/dom/Document.cpp 2015-10-18 22:15:18 UTC (rev 191262)
@@ -1954,15 +1954,15 @@
m_ignorePendingStylesheets = oldIgnore;
}
-Ref<RenderStyle> Document::styleForElementIgnoringPendingStylesheets(Element* element)
+Ref<RenderStyle> Document::styleForElementIgnoringPendingStylesheets(Element& element, RenderStyle* parentStyle)
{
- ASSERT_ARG(element, &element->document() == this);
+ ASSERT(&element.document() == this);
// On iOS request delegates called during styleForElement may result in re-entering WebKit and killing the style resolver.
ResourceLoadScheduler::Suspender suspender(*platformStrategies()->loaderStrategy()->resourceLoadScheduler());
TemporaryChange<bool> change(m_ignorePendingStylesheets, true);
- return ensureStyleResolver().styleForElement(element, element->parentNode() ? element->parentNode()->computedStyle() : nullptr);
+ return element.resolveStyle(parentStyle);
}
bool Document::updateLayoutIfDimensionsOutOfDate(Element& element, DimensionsCheck dimensionsCheck)
Modified: trunk/Source/WebCore/dom/Document.h (191261 => 191262)
--- trunk/Source/WebCore/dom/Document.h 2015-10-18 17:19:40 UTC (rev 191261)
+++ trunk/Source/WebCore/dom/Document.h 2015-10-18 22:15:18 UTC (rev 191262)
@@ -570,7 +570,7 @@
};
WEBCORE_EXPORT void updateLayoutIgnorePendingStylesheets(RunPostLayoutTasks = RunPostLayoutTasks::Asynchronously);
- Ref<RenderStyle> styleForElementIgnoringPendingStylesheets(Element*);
+ Ref<RenderStyle> styleForElementIgnoringPendingStylesheets(Element&, RenderStyle* parentStyle);
// Returns true if page box (margin boxes and page borders) is visible.
WEBCORE_EXPORT bool isPageBoxVisible(int pageIndex);
Modified: trunk/Source/WebCore/dom/Element.cpp (191261 => 191262)
--- trunk/Source/WebCore/dom/Element.cpp 2015-10-18 17:19:40 UTC (rev 191261)
+++ trunk/Source/WebCore/dom/Element.cpp 2015-10-18 22:15:18 UTC (rev 191262)
@@ -33,6 +33,7 @@
#include "ChromeClient.h"
#include "ClientRect.h"
#include "ClientRectList.h"
+#include "ComposedTreeAncestorIterator.h"
#include "ContainerNodeAlgorithms.h"
#include "DOMTokenList.h"
#include "Dictionary.h"
@@ -2434,44 +2435,79 @@
ensureElementRareData().setMinimumSizeForResizing(size);
}
-static PseudoElement* beforeOrAfterPseudoElement(Element* host, PseudoId pseudoElementSpecifier)
+static PseudoElement* beforeOrAfterPseudoElement(Element& host, PseudoId pseudoElementSpecifier)
{
switch (pseudoElementSpecifier) {
case BEFORE:
- return host->beforePseudoElement();
+ return host.beforePseudoElement();
case AFTER:
- return host->afterPseudoElement();
+ return host.afterPseudoElement();
default:
- return 0;
+ return nullptr;
}
}
+RenderStyle* Element::existingComputedStyle()
+{
+ if (auto* renderTreeStyle = renderStyle())
+ return renderTreeStyle;
+
+ if (hasRareData())
+ return elementRareData()->computedStyle();
+
+ return nullptr;
+}
+
+RenderStyle& Element::resolveComputedStyle()
+{
+ ASSERT(inDocument());
+ ASSERT(!existingComputedStyle());
+
+ Deque<Element*, 32> elementsRequiringComputedStyle({ this });
+ RenderStyle* computedStyle = nullptr;
+
+ // Collect ancestors until we find one that has style.
+ auto composedAncestors = composedTreeAncestors(*this);
+ for (auto& ancestor : composedAncestors) {
+ if (!is<Element>(ancestor))
+ break;
+ auto& ancestorElement = downcast<Element>(ancestor);
+ elementsRequiringComputedStyle.prepend(&ancestorElement);
+ if (auto* existingStyle = ancestorElement.existingComputedStyle()) {
+ computedStyle = existingStyle;
+ break;
+ }
+ }
+
+ // Resolve and cache styles starting from the most distant ancestor.
+ for (auto* element : elementsRequiringComputedStyle) {
+ auto style = document().styleForElementIgnoringPendingStylesheets(*element, computedStyle);
+ computedStyle = style.ptr();
+ ElementRareData& rareData = element->ensureElementRareData();
+ rareData.setComputedStyle(WTF::move(style));
+ }
+
+ return *computedStyle;
+}
+
RenderStyle* Element::computedStyle(PseudoId pseudoElementSpecifier)
{
- if (PseudoElement* pseudoElement = beforeOrAfterPseudoElement(this, pseudoElementSpecifier))
+ if (PseudoElement* pseudoElement = beforeOrAfterPseudoElement(*this, pseudoElementSpecifier))
return pseudoElement->computedStyle();
- // FIXME: Find and use the renderer from the pseudo element instead of the actual element so that the 'length'
- // properties, which are only known by the renderer because it did the layout, will be correct and so that the
- // values returned for the ":selection" pseudo-element will be correct.
- if (RenderStyle* usedStyle = renderStyle()) {
- if (pseudoElementSpecifier) {
- RenderStyle* cachedPseudoStyle = usedStyle->getCachedPseudoStyle(pseudoElementSpecifier);
- return cachedPseudoStyle ? cachedPseudoStyle : usedStyle;
- }
- return usedStyle;
+ auto* style = existingComputedStyle();
+ if (!style) {
+ if (!inDocument())
+ return nullptr;
+ style = &resolveComputedStyle();
}
- if (!inDocument()) {
- // FIXME: Try to do better than this. Ensure that styleForElement() works for elements that are not in the
- // document tree and figure out when to destroy the computed style for such elements.
- return nullptr;
+ if (pseudoElementSpecifier) {
+ if (auto* cachedPseudoStyle = style->getCachedPseudoStyle(pseudoElementSpecifier))
+ return cachedPseudoStyle;
}
- ElementRareData& data = ""
- if (!data.computedStyle())
- data.setComputedStyle(document().styleForElementIgnoringPendingStylesheets(this));
- return pseudoElementSpecifier ? data.computedStyle()->getCachedPseudoStyle(pseudoElementSpecifier) : data.computedStyle();
+ return style;
}
void Element::setStyleAffectedByEmpty()
Modified: trunk/Source/WebCore/dom/Element.h (191261 => 191262)
--- trunk/Source/WebCore/dom/Element.h 2015-10-18 17:19:40 UTC (rev 191261)
+++ trunk/Source/WebCore/dom/Element.h 2015-10-18 22:15:18 UTC (rev 191262)
@@ -573,6 +573,9 @@
void removeShadowRoot();
+ RenderStyle* existingComputedStyle();
+ RenderStyle& resolveComputedStyle();
+
bool rareDataStyleAffectedByEmpty() const;
bool rareDataChildrenAffectedByHover() const;
bool rareDataChildrenAffectedByActive() const;
Modified: trunk/Source/WebCore/dom/Node.cpp (191261 => 191262)
--- trunk/Source/WebCore/dom/Node.cpp 2015-10-18 17:19:40 UTC (rev 191261)
+++ trunk/Source/WebCore/dom/Node.cpp 2015-10-18 22:15:18 UTC (rev 191262)
@@ -1006,11 +1006,10 @@
RenderStyle* Node::computedStyle(PseudoId pseudoElementSpecifier)
{
- for (Node* node = this; node; node = node->parentOrShadowHostNode()) {
- if (is<Element>(*node))
- return downcast<Element>(*node).computedStyle(pseudoElementSpecifier);
- }
- return nullptr;
+ auto* composedParent = composedTreeAncestors(*this).first();
+ if (!composedParent)
+ return nullptr;
+ return composedParent->computedStyle(pseudoElementSpecifier);
}
int Node::maxCharacterOffset() const
Modified: trunk/Source/WebCore/html/HTMLSelectElement.cpp (191261 => 191262)
--- trunk/Source/WebCore/html/HTMLSelectElement.cpp 2015-10-18 17:19:40 UTC (rev 191261)
+++ trunk/Source/WebCore/html/HTMLSelectElement.cpp 2015-10-18 22:15:18 UTC (rev 191262)
@@ -887,13 +887,14 @@
downcast<HTMLOptionElement>(*element).setSelectedState(true);
}
+ updateValidity();
+
// For the menu list case, this is what makes the selected element appear.
if (auto* renderer = this->renderer())
renderer->updateFromElement();
scrollToSelection();
- updateValidity();
if (usesMenuList()) {
m_isProcessingUserDrivenChange = flags & UserDriven;
if (flags & DispatchChangeEvent)
