<rdar://problem/9750062> REGRESSION: Button text missing in many iTunes Store pages
authormitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 Jul 2011 15:37:30 +0000 (15:37 +0000)
committermitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 Jul 2011 15:37:30 +0000 (15:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=64236

Reviewed by Maciej Stachowiak.

Source/WebCore:

Test: fast/css/empty-display-none.html

When an :empty selector caused an element to not have a renderer, the check for empty style
change when finishing parsing the elemenet did nothing, because it could not check if the
element’s current style was affected by :empty. The fix is to record the fact that the style
was affected by :empty in ElementRareData in the no-renderer case.

* dom/Element.cpp:
(WebCore::Element::recalcStyle): Clear the m_styleAffectedByEmpty flag.
(WebCore::checkForEmptyStyleChange): If the style is null (meaning there is no renderer), check
Element::styleAffectedByEmpty().
(WebCore::Element::setStyleAffectedByEmpty): Added. Sets the flag in rare data.
(WebCore::Element::styleAffectedByEmpty): Added. Checks for the flag in rare data.
* dom/Element.h:
* dom/ElementRareData.h:
(WebCore::ElementRareData::ElementRareData): Added m_styleAffectedByEmpty and initialized it
to false.
* dom/NodeRenderingContext.cpp:
(WebCore::NodeRendererFactory::createRendererAndStyle): If an element doesn’t need a renderer
and its style is affected by :empty, record this fact in the element by calling setStyleAffectedByEmpty().

LayoutTests:

* fast/css/empty-display-none-expected.txt: Added.
* fast/css/empty-display-none.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@90691 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/css/empty-display-none-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/empty-display-none.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/Element.h
Source/WebCore/dom/ElementRareData.h
Source/WebCore/dom/NodeRenderingContext.cpp

index 268dd5b..a27944e 100644 (file)
@@ -1,3 +1,13 @@
+2011-07-10  Dan Bernstein  <mitz@apple.com>
+
+        <rdar://problem/9750062> REGRESSION: Button text missing in many iTunes Store pages
+        https://bugs.webkit.org/show_bug.cgi?id=64236
+
+        Reviewed by Maciej Stachowiak.
+
+        * fast/css/empty-display-none-expected.txt: Added.
+        * fast/css/empty-display-none.html: Added.
+
 2011-07-08  Stephen White  <senorblanco@chromium.org>
 
         Unreviewed; new chromium GPU pixel results for overflow-scroll-expected.
diff --git a/LayoutTests/fast/css/empty-display-none-expected.txt b/LayoutTests/fast/css/empty-display-none-expected.txt
new file mode 100644 (file)
index 0000000..c95677c
--- /dev/null
@@ -0,0 +1,3 @@
+This tests that specifying display: none; for the :empty pseudoclass doesn’t affect non-empty elements.
+
+PASS
diff --git a/LayoutTests/fast/css/empty-display-none.html b/LayoutTests/fast/css/empty-display-none.html
new file mode 100644 (file)
index 0000000..13115ab
--- /dev/null
@@ -0,0 +1,18 @@
+<style>
+    div { width: 100px; height: 100px; }
+    #target { margin-top: -100px; background-color: green; }
+    #target:empty { display: none; }
+</style>
+<p>
+    This tests that specifying <tt>display: none;</tt> for the <tt>:empty</tt>
+    pseudoclass doesn&rsquo;t affect non-empty elements.
+</p>
+<div style="background-color: red;"></div>
+<div id="target"><span></span></div>
+<p id="result"></p>
+<script>
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+
+    document.getElementById("result").innerText = document.getElementById("target").offsetWidth === 100 ? "PASS" : "FAIL";
+</script>
index b0b5c8f..b80d538 100644 (file)
@@ -1,3 +1,31 @@
+2011-07-10  Dan Bernstein  <mitz@apple.com>
+
+        <rdar://problem/9750062> REGRESSION: Button text missing in many iTunes Store pages
+        https://bugs.webkit.org/show_bug.cgi?id=64236
+
+        Reviewed by Maciej Stachowiak.
+
+        Test: fast/css/empty-display-none.html
+
+        When an :empty selector caused an element to not have a renderer, the check for empty style
+        change when finishing parsing the elemenet did nothing, because it could not check if the
+        element’s current style was affected by :empty. The fix is to record the fact that the style
+        was affected by :empty in ElementRareData in the no-renderer case.
+
+        * dom/Element.cpp:
+        (WebCore::Element::recalcStyle): Clear the m_styleAffectedByEmpty flag.
+        (WebCore::checkForEmptyStyleChange): If the style is null (meaning there is no renderer), check
+        Element::styleAffectedByEmpty().
+        (WebCore::Element::setStyleAffectedByEmpty): Added. Sets the flag in rare data.
+        (WebCore::Element::styleAffectedByEmpty): Added. Checks for the flag in rare data.
+        * dom/Element.h:
+        * dom/ElementRareData.h:
+        (WebCore::ElementRareData::ElementRareData): Added m_styleAffectedByEmpty and initialized it
+        to false.
+        * dom/NodeRenderingContext.cpp:
+        (WebCore::NodeRendererFactory::createRendererAndStyle): If an element doesn’t need a renderer
+        and its style is affected by :empty, record this fact in the element by calling setStyleAffectedByEmpty().
+
 2011-07-10  Mark Rowe  <mrowe@apple.com>
 
         Fix the build.
index 94225cb..79eea9e 100644 (file)
@@ -1102,8 +1102,11 @@ void Element::recalcStyle(StyleChange change)
     bool hasIndirectAdjacentRules = currentStyle && currentStyle->childrenAffectedByForwardPositionalRules();
 
     if ((change > NoChange || needsStyleRecalc())) {
-        if (hasRareData())
-            rareData()->resetComputedStyle();
+        if (hasRareData()) {
+            ElementRareData* data = rareData();
+            data->resetComputedStyle();
+            data->m_styleAffectedByEmpty = false;
+        }
     }
     if (hasParentStyle && (change >= Inherit || needsStyleRecalc())) {
         RefPtr<RenderStyle> newStyle = styleForRenderer(NodeRenderingContext(this, 0));
@@ -1301,10 +1304,10 @@ bool Element::childTypeAllowed(NodeType type) const
 
 static void checkForEmptyStyleChange(Element* element, RenderStyle* style)
 {
-    if (!style)
+    if (!style && !element->styleAffectedByEmpty())
         return;
 
-    if (style->affectedByEmpty() && (!style->emptyState() || element->hasChildNodes()))
+    if (!style || (style->affectedByEmpty() && (!style->emptyState() || element->hasChildNodes())))
         element->setNeedsStyleRecalc();
 }
 
@@ -1750,6 +1753,17 @@ RenderStyle* Element::computedStyle(PseudoId pseudoElementSpecifier)
     return pseudoElementSpecifier ? data->m_computedStyle->getCachedPseudoStyle(pseudoElementSpecifier) : data->m_computedStyle.get();
 }
 
+void Element::setStyleAffectedByEmpty()
+{
+    ElementRareData* data = ensureRareData();
+    data->m_styleAffectedByEmpty = true;
+}
+
+bool Element::styleAffectedByEmpty() const
+{
+    return hasRareData() && rareData()->m_styleAffectedByEmpty;
+}
+
 AtomicString Element::computeInheritedLanguage() const
 {
     const Node* n = this;
index 94432b4..d389bfe 100644 (file)
@@ -240,6 +240,9 @@ public:
 
     RenderStyle* computedStyle(PseudoId = NOPSEUDO);
 
+    void setStyleAffectedByEmpty();
+    bool styleAffectedByEmpty() const;
+
     AtomicString computeInheritedLanguage() const;
 
     void dispatchAttrRemovalEvent(Attribute*);
index 7277a6f..8301505 100644 (file)
@@ -50,6 +50,8 @@ public:
     OwnPtr<DatasetDOMStringMap> m_datasetDOMStringMap;
     OwnPtr<ClassList> m_classList;
 
+    bool m_styleAffectedByEmpty;
+
 #if ENABLE(FULLSCREEN_API)
     bool m_containsFullScreenElement;
 #endif
@@ -63,6 +65,7 @@ inline IntSize defaultMinimumSizeForResizing()
 inline ElementRareData::ElementRareData()
     : m_minimumSizeForResizing(defaultMinimumSizeForResizing())
     , m_shadowRoot(0)
+    , m_styleAffectedByEmpty(false)
 #if ENABLE(FULLSCREEN_API)
     , m_containsFullScreenElement(false)
 #endif
index 69ec1bc..155a366 100644 (file)
@@ -271,8 +271,14 @@ RenderObject* NodeRendererFactory::createRendererAndStyle()
         return 0;
 
     m_context.setStyle(node->styleForRenderer(m_context));
-    if (!node->rendererIsNeeded(m_context))
+    if (!node->rendererIsNeeded(m_context)) {
+        if (node->isElementNode()) {
+            Element* element = toElement(node);
+            if (m_context.style()->affectedByEmpty())
+                element->setStyleAffectedByEmpty();
+        }
         return 0;
+    }
 
     RenderObject* newRenderer = node->createRenderer(document->renderArena(), m_context.style());
     if (!newRenderer)