[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373

eric at webkit.org eric at webkit.org
Thu Apr 8 01:10:28 UTC 2010


The following commit has been merged in the webkit-1.2 branch:
commit ae29b2f4e6a82f24f3b245cd8eff847c9395e371
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Sat Jan 16 02:14:25 2010 +0000

    2010-01-15  Carol Szabo  <carol.szabo at nokia.com>
    
            Reviewed by Darin Adler.
    
            CSS2.1 Counters not updated when new elements are inserted in the DOM.
            https://bugs.webkit.org/show_bug.cgi?id=32884
    
            * fast/css/counters/adding-nodes-expected.txt: Added.
            * fast/css/counters/adding-nodes.html: Added.
    2010-01-15  Carol Szabo  <carol.szabo at nokia.com>
    
            Reviewed by Darin Adler.
    
            CSS2.1 Counters not updated when new elements are inserted in the DOM.
            https://bugs.webkit.org/show_bug.cgi?id=32884
    
            Test: fast/css/counters/adding-nodes.html
    
            * rendering/CounterNode.cpp:
            (WebCore::CounterNode::insertAfter):
            Modified to handle the addition of nodes with children. Needed when formerly
            root nodes become descendants of a new node.
            * rendering/RenderCounter.cpp:
            (WebCore::makeCounterNode):
            Changed to handle the case when root counter nodes lose their root
            status as a result of a new root counter node creation.
            (WebCore::destroyCounterNodeWithoutMapRemoval):
            Refactored more code into destroyCounterNodeChildren and renamed the
            function according to its new action.
            (WebCore::RenderCounter::destroyCounterNodes):
            Simplified to share more code with the new destroyCounterNode.
            (WebCore::RenderCounter::destroyCounterNode):
            Added to allow for selective counterNode destruction.
            (WebCore::RenderCounter::rendererSubtreeAttached):
            Added to refresh counter values in response to DOM changes.
            For renderers with no attached counters the execution time of this
            function cannot be discerned in comparison with the time needed to
            add a node or change the style of a node.
            (WebCore::updateCounters):
            Helper function for rendererSubtreeAttached. Updates the counters
            attached to a Renderer in response to the renderer or its ancestors
            being attached to the renderer tree.
            * rendering/RenderCounter.h:
            * rendering/RenderObject.cpp:
            (WebCore::RenderObject::addChild):
            Changed to update counter values if needed.
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@53355 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index b263738..726b833 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2010-01-15  Carol Szabo  <carol.szabo at nokia.com>
+
+        Reviewed by Darin Adler.
+
+        CSS2.1 Counters not updated when new elements are inserted in the DOM.
+        https://bugs.webkit.org/show_bug.cgi?id=32884
+
+        * fast/css/counters/adding-nodes-expected.txt: Added.
+        * fast/css/counters/adding-nodes.html: Added.
+
 2010-01-15  Darin Fisher  <darin at chromium.org>
 
         Fix flakey test.
diff --git a/LayoutTests/fast/css/counters/adding-nodes-expected.txt b/LayoutTests/fast/css/counters/adding-nodes-expected.txt
new file mode 100644
index 0000000..6098376
--- /dev/null
+++ b/LayoutTests/fast/css/counters/adding-nodes-expected.txt
@@ -0,0 +1,4 @@
+The following two lines should have the same content:
+
+1-2-2.0-2.0.1-
+1-2-2.0-2.0.1-
diff --git a/LayoutTests/fast/css/counters/adding-nodes.html b/LayoutTests/fast/css/counters/adding-nodes.html
new file mode 100644
index 0000000..3ab498b
--- /dev/null
+++ b/LayoutTests/fast/css/counters/adding-nodes.html
@@ -0,0 +1,55 @@
+<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN">
+<html><head>
+    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
+    <title>Adding nodes that affect the CSS counter tree</title>
+    <link rel="author" href="mailto:carol.szabo at nokia.com" title="Carol Szabo">
+    <link rel="help" href="http://www.w3.org/TR/CSS21/generate.html#counters">
+    <link rel="help" href="http://www.w3.org/TR/CSS21/generate.html#propdef-content">
+    <link rel="help" href="http://www.w3.org/TR/CSS21/syndata.html#counter">
+    <meta content="dom" name="flags">
+    <meta http-equiv="Content-Style-Type" content="text/css">
+    <meta http-equiv="Content-Script-Type" content="text/javascript">
+    <style type="text/css">
+        body { white-space: nowrap; }
+        .reset { counter-reset: c; }
+        .increment:before, .use:before { content: counters(c, ".") "-"; }
+        .increment { counter-increment: c; }
+    </style>
+    <script type="text/javascript">
+        if (window.layoutTestController) {
+            layoutTestController.dumpAsText();
+            layoutTestController.waitUntilDone();
+        }
+
+        function run() {
+            testElement = document.getElementById("test");
+            newSpanElement = document.createElement("span");
+            newSpanElement.setAttribute("class","increment");
+            newSpanElement.setAttribute("id","newNode");
+            testElement.insertBefore(newSpanElement, testElement.firstChild);
+            if (window.layoutTestController) {
+                console = document.getElementById("console");
+                spanList = testElement.getElementsByTagName("span")
+                for (i = 0; i < spanList.length; ++i ) {
+                    newSpanElement = document.createElement("span");
+                    id = spanList.item(i).getAttribute("id");
+                    if (id != null)
+                        newSpanElement.innerText = layoutTestController.counterValueForElementById(id);
+                    if (newSpanElement.innerText.length)
+                        newSpanElement.innerText = newSpanElement.innerText + "-";
+                    console.appendChild(newSpanElement);
+                }
+                testElement.parentNode.removeChild(testElement);
+                testElement = document.getElementById("reference");
+                testElement.innerHTML = testElement.innerHTML.replace(/<b>a<u>b<\/u><\/b>/g,"");
+                layoutTestController.notifyDone();
+            }
+        }
+    </script>
+</head><body onload="setTimeout('run()', 0)">
+    <p>The following two lines should have the same content:</p>
+    <div id="test"><b>a<u>b</u></b><b>a<u>b</u></b><span id="parent"><b>a<u>b</u></b><span id="rootOne" class="increment"/><b>a<u>b</u></b></span><b>a<u>b</u></b><span id="rootTwo" class="reset"><b>a<u>b</u></b></span><b>a<u>b</u></b><span id="r2c1" class="use"><b>a<u>b</u></b><span id="nrreset" class="reset"><b>a<u>b</u></b><span id="r3c1" class="increment"></span><b>a<u>b</u></b></span></span></div>
+    <div id="reference">1-<b>a<u>b</u></b><b>a<u>b</u></b><b>a<u>b</u></b>2-<b>a<u>b</u></b><b>a<u>b</u></b><b>a<u>b</u></b><b>a<u>b</u></b>2.0-<b>a<u>b</u></b><b>a<u>b</u></b>2.0.1-<b>a<u>b</u></b></div>
+    <hr>
+    <div id="console"></div>
+</body></html>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 39f7c8e..59de5b0 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,41 @@
+2010-01-15  Carol Szabo  <carol.szabo at nokia.com>
+
+        Reviewed by Darin Adler.
+
+        CSS2.1 Counters not updated when new elements are inserted in the DOM.
+        https://bugs.webkit.org/show_bug.cgi?id=32884
+
+        Test: fast/css/counters/adding-nodes.html
+
+        * rendering/CounterNode.cpp:
+        (WebCore::CounterNode::insertAfter):
+        Modified to handle the addition of nodes with children. Needed when formerly 
+        root nodes become descendants of a new node.
+        * rendering/RenderCounter.cpp:
+        (WebCore::makeCounterNode):
+        Changed to handle the case when root counter nodes lose their root 
+        status as a result of a new root counter node creation. 
+        (WebCore::destroyCounterNodeWithoutMapRemoval):
+        Refactored more code into destroyCounterNodeChildren and renamed the
+        function according to its new action.
+        (WebCore::RenderCounter::destroyCounterNodes):
+        Simplified to share more code with the new destroyCounterNode.
+        (WebCore::RenderCounter::destroyCounterNode):
+        Added to allow for selective counterNode destruction.
+        (WebCore::RenderCounter::rendererSubtreeAttached):
+        Added to refresh counter values in response to DOM changes.
+        For renderers with no attached counters the execution time of this
+        function cannot be discerned in comparison with the time needed to
+        add a node or change the style of a node.
+        (WebCore::updateCounters):
+        Helper function for rendererSubtreeAttached. Updates the counters
+        attached to a Renderer in response to the renderer or its ancestors
+        being attached to the renderer tree.
+        * rendering/RenderCounter.h:
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::addChild):
+        Changed to update counter values if needed.
+
 2010-01-15  Alejandro G. Castro  <alex at igalia.com>
 
         Reviewed by Xan Lopez.
diff --git a/WebCore/rendering/CounterNode.cpp b/WebCore/rendering/CounterNode.cpp
index e9350e4..c164c81 100644
--- a/WebCore/rendering/CounterNode.cpp
+++ b/WebCore/rendering/CounterNode.cpp
@@ -22,16 +22,10 @@
 #include "config.h"
 #include "CounterNode.h"
 
+#include "RenderCounter.h"
 #include "RenderObject.h"
 #include <stdio.h>
 
-// FIXME: There's currently no strategy for getting the counter tree updated when new
-// elements with counter-reset and counter-increment styles are added to the render tree.
-// Also, the code can't handle changes where an existing node needs to change into a
-// "reset" node, or from a "reset" node back to not a "reset" node. As of this writing,
-// at least some of these problems manifest as failures in the t1204-increment and
-// t1204-reset tests in the CSS 2.1 test suite.
-
 namespace WebCore {
 
 CounterNode::CounterNode(RenderObject* o, bool hasResetType, int value)
@@ -140,6 +134,11 @@ void CounterNode::insertAfter(CounterNode* newChild, CounterNode* refChild, cons
     ASSERT(!newChild->m_nextSibling);
     ASSERT(!refChild || refChild->m_parent == this);
 
+    if (newChild->m_hasResetType) {
+        while (m_lastChild != refChild)
+            RenderCounter::destroyCounterNode(m_lastChild->renderer(), identifier);
+    }
+
     CounterNode* next;
 
     if (refChild) {
@@ -150,21 +149,57 @@ void CounterNode::insertAfter(CounterNode* newChild, CounterNode* refChild, cons
         m_firstChild = newChild;
     }
 
-    if (next) {
-        ASSERT(next->m_previousSibling == refChild);
-        next->m_previousSibling = newChild;
-    } else {
-        ASSERT(m_lastChild == refChild);
-        m_lastChild = newChild;
-    }
-
     newChild->m_parent = this;
     newChild->m_previousSibling = refChild;
-    newChild->m_nextSibling = next;
 
-    newChild->m_countInParent = newChild->computeCountInParent();
+    if (!newChild->m_firstChild || newChild->m_hasResetType) {
+        newChild->m_nextSibling = next;
+        if (next) {
+            ASSERT(next->m_previousSibling == refChild);
+            next->m_previousSibling = newChild;
+        } else {
+            ASSERT(m_lastChild == refChild);
+            m_lastChild = newChild;
+        }
+
+        newChild->m_countInParent = newChild->computeCountInParent();
+        newChild->resetRenderers(identifier);
+        if (next)
+            next->recount(identifier);
+        return;
+    }
+
+    // The code below handles the case when a formerly root increment counter is loosing its root position
+    // and therefore its children become next siblings.
+    CounterNode* last = newChild->m_lastChild;
+    CounterNode* first = newChild->m_firstChild;
+
+    newChild->m_nextSibling = first;
+    first->m_previousSibling = newChild;
+    // The case when the original next sibling of the inserted node becomes a child of
+    // one of the former children of the inserted node is not handled as it is believed
+    // to be impossible since:
+    // 1. if the increment counter node lost it's root position as a result of another
+    //    counter node being created, it will be inserted as the last child so next is null.
+    // 2. if the increment counter node lost it's root position as a result of a renderer being
+    //    inserted into the document's render tree, all its former children counters are attached
+    //    to children of the inserted renderer and hence cannot be in scope for counter nodes
+    //    attached to renderers that were already in the document's render tree.
+    last->m_nextSibling = next;
     if (next)
-        next->recount(identifier);
+        next->m_previousSibling = last;
+    else
+        m_lastChild = last;
+    for (next = first; ; next = next->m_nextSibling) {
+        next->m_parent = this;
+        if (last == next)
+            break;
+    }
+    newChild->m_firstChild = 0;
+    newChild->m_lastChild = 0;
+    newChild->m_countInParent = newChild->computeCountInParent();
+    newChild->resetRenderer(identifier);
+    first->recount(identifier);
 }
 
 void CounterNode::removeChild(CounterNode* oldChild, const AtomicString& identifier)
diff --git a/WebCore/rendering/RenderCounter.cpp b/WebCore/rendering/RenderCounter.cpp
index f88ab29..ebf5314 100644
--- a/WebCore/rendering/RenderCounter.cpp
+++ b/WebCore/rendering/RenderCounter.cpp
@@ -254,6 +254,29 @@ static CounterNode* makeCounterNode(RenderObject* object, const AtomicString& id
         object->m_hasCounterNodeMap = true;
     }
     nodeMap->set(identifier.impl(), newNode);
+    if (newNode->parent() || !object->nextInPreOrder(object->parent()))
+        return newNode;
+    // Checking if some nodes that were previously counter tree root nodes
+    // should become children of this node now.
+    CounterMaps& maps = counterMaps();
+    RenderObject* stayWithin = object->parent();
+    for (RenderObject* currentRenderer = object->nextInPreOrder(stayWithin); currentRenderer; currentRenderer = currentRenderer->nextInPreOrder(stayWithin)) {
+        if (!currentRenderer->m_hasCounterNodeMap)
+            continue;
+        CounterNode* currentCounter = maps.get(currentRenderer)->get(identifier.impl());
+        if (!currentCounter)
+            continue;
+        if (currentCounter->parent()) {
+            ASSERT(newNode->firstChild());
+            if (currentRenderer->lastChild())
+                currentRenderer = currentRenderer->lastChild();
+            continue;
+        }
+        if (stayWithin != currentRenderer->parent() || !currentCounter->hasResetType())
+            newNode->insertAfter(currentCounter, newNode->lastChild(), identifier);
+        if (currentRenderer->lastChild())
+            currentRenderer = currentRenderer->lastChild();
+    }
     return newNode;
 }
 
@@ -314,7 +337,7 @@ void RenderCounter::invalidate(const AtomicString& identifier)
     setNeedsLayoutAndPrefWidthsRecalc();
 }
 
-static void destroyCounterNodeChildren(const AtomicString& identifier, CounterNode* node)
+static void destroyCounterNodeWithoutMapRemoval(const AtomicString& identifier, CounterNode* node)
 {
     CounterNode* previous;
     for (CounterNode* child = node->lastDescendant(); child && child != node; child = previous) {
@@ -329,27 +352,92 @@ static void destroyCounterNodeChildren(const AtomicString& identifier, CounterNo
         }
         delete child;
     }
+    RenderObject* renderer = node->renderer();
+    if (!renderer->documentBeingDestroyed()) {
+        if (RenderObjectChildList* children = renderer->virtualChildren())
+            children->invalidateCounters(renderer, identifier);
+    }
+    if (CounterNode* parent = node->parent())
+        parent->removeChild(node, identifier);
+    delete node;
 }
 
-void RenderCounter::destroyCounterNodes(RenderObject* object)
+void RenderCounter::destroyCounterNodes(RenderObject* renderer)
 {
     CounterMaps& maps = counterMaps();
-    CounterMap* map = maps.get(object);
-    if (!map)
+    CounterMaps::iterator mapsIterator = maps.find(renderer);
+    if (mapsIterator == maps.end())
         return;
-    maps.remove(object);
-
+    CounterMap* map = mapsIterator->second;
     CounterMap::const_iterator end = map->end();
     for (CounterMap::const_iterator it = map->begin(); it != end; ++it) {
-        CounterNode* node = it->second;
         AtomicString identifier(it->first.get());
-        destroyCounterNodeChildren(identifier, node);
-        if (CounterNode* parent = node->parent())
-            parent->removeChild(node, identifier);
-        delete node;
+        destroyCounterNodeWithoutMapRemoval(identifier, it->second);
     }
-
+    maps.remove(mapsIterator);
     delete map;
+    renderer->m_hasCounterNodeMap = false;
+}
+
+void RenderCounter::destroyCounterNode(RenderObject* renderer, const AtomicString& identifier)
+{
+    CounterMap* map = counterMaps().get(renderer);
+    if (!map)
+        return;
+    CounterMap::iterator mapIterator = map->find(identifier.impl());
+    if (mapIterator == map->end())
+        return;
+    destroyCounterNodeWithoutMapRemoval(identifier, mapIterator->second);
+    map->remove(mapIterator);
+    // We do not delete "map" here even if empty because we expect to reuse
+    // it soon. In order for a renderer to lose all its counters permanently,
+    // a style change for the renderer involving removal of all counter
+    // directives must occur, in which case, RenderCounter::destroyCounterNodes()
+    // must be called.
+    // The destruction of the Renderer (possibly caused by the removal of its 
+    // associated DOM node) is the other case that leads to the permanent
+    // destruction of all counters attached to a Renderer. In this case
+    // RenderCounter::destroyCounterNodes() must be and is now called, too.
+    // RenderCounter::destroyCounterNodes() handles destruction of the counter
+    // map associated with a renderer, so there is no risk in leaking the map.
+}
+
+static void updateCounters(RenderObject* renderer)
+{
+    ASSERT(renderer->style());
+    const CounterDirectiveMap* directiveMap = renderer->style()->counterDirectives();
+    if (!directiveMap)
+        return;
+    CounterDirectiveMap::const_iterator end = directiveMap->end();
+    if (!renderer->m_hasCounterNodeMap) {
+        for (CounterDirectiveMap::const_iterator it = directiveMap->begin(); it != end; ++it)
+            makeCounterNode(renderer, AtomicString(it->first.get()), false);
+        return;
+    }
+    CounterMap* counterMap = counterMaps().get(renderer);
+    ASSERT(counterMap);
+    for (CounterDirectiveMap::const_iterator it = directiveMap->begin(); it != end; ++it) {
+        CounterNode* node = counterMap->get(it->first.get());
+        if (!node) {
+            makeCounterNode(renderer, AtomicString(it->first.get()), false);
+            continue;
+        }
+        CounterNode* newParent = 0;
+        CounterNode* newPreviousSibling;
+        findPlaceForCounter(renderer, AtomicString(it->first.get()), node->hasResetType(), newParent, newPreviousSibling);
+        CounterNode* parent = node->parent();
+        if (newParent == parent && newPreviousSibling == node->previousSibling())
+            continue;
+        if (parent)
+            parent->removeChild(node, it->first.get());
+        newParent->insertAfter(node, newPreviousSibling, it->first.get());
+    }
+}
+
+void RenderCounter::rendererSubtreeAttached(RenderObject* renderer)
+{
+    for (RenderObject* descendant = renderer; descendant; descendant = descendant->nextInPreOrder(renderer))
+        updateCounters(descendant);
 }
 
 } // namespace WebCore
diff --git a/WebCore/rendering/RenderCounter.h b/WebCore/rendering/RenderCounter.h
index 356f1bd..dec4425 100644
--- a/WebCore/rendering/RenderCounter.h
+++ b/WebCore/rendering/RenderCounter.h
@@ -40,6 +40,8 @@ public:
     void invalidate(const AtomicString& identifier);
 
     static void destroyCounterNodes(RenderObject*);
+    static void destroyCounterNode(RenderObject*, const AtomicString& identifier);
+    static void rendererSubtreeAttached(RenderObject*);
 
 private:
     virtual const char* renderName() const;
diff --git a/WebCore/rendering/RenderObject.cpp b/WebCore/rendering/RenderObject.cpp
index efb3051..74d0ef3 100644
--- a/WebCore/rendering/RenderObject.cpp
+++ b/WebCore/rendering/RenderObject.cpp
@@ -309,7 +309,7 @@ void RenderObject::addChild(RenderObject* newChild, RenderObject* beforeChild)
         // Just add it...
         children->insertChildNode(this, newChild, beforeChild);
     }
-    
+    RenderCounter::rendererSubtreeAttached(newChild);
     if (newChild->isText() && newChild->style()->textTransform() == CAPITALIZE) {
         RefPtr<StringImpl> textToTransform = toRenderText(newChild)->originalText();
         if (textToTransform)

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list