Title: [148863] trunk
Revision
148863
Author
abu...@adobe.com
Date
2013-04-22 01:32:28 -0700 (Mon, 22 Apr 2013)

Log Message

Use DOM ordering for list counts
https://bugs.webkit.org/show_bug.cgi?id=110352

Reviewed by Elliott Sprehn.

Source/WebCore:

Currently the list items ordering is made by traversing the render tree. This gives bad results for:
- list items flown inside regions because they are not rendered as descendants of their DOM list ancestors.
- list items matched to insertion points inside a shadow tree. The insertion point may be a child of a
list so the numbering gets broken.

To implement correct DOM ordering I've taken into account the fact that pseudo-elements can have display: list-item
so they should be included when traversing the DOM tree. I've added helper methods on the NodeTraversal
namespace that should allow easily traversing the tree including the pseudo-elements (e.g. firstChildWithPseudo
for an element with pseudo-before will return the before PseudoElement). Using these helper methods I've implemented
pre-order traversal methods aware of the pseudo-elements.
An effect of this change is how the list items inside shadow tree update their counting. With the patch, if there's
no <ol> or <ul> element on the ancestor chain of a <li> element to the shadow root, the list node will be considered the
first parent of the <li> or the shadow root if there is no ancestor.
The RenderListItem class now makes use of this new method of traversal, replacing the one based on the render tree.
To simplify the CSS counters implementation, I've changed the traversal functions inside RenderCounter to also make use
of this method. The CSS counters and the list items now have the same traversal algorithm.

In later patches I'll add tests that should cover the regions and shadow DOM use cases.
Tests bug for shadow DOM: https://bugs.webkit.org/show_bug.cgi?id=113193
Tests bug for regions: https://bugs.webkit.org/show_bug.cgi?id=103975

Tests: no new tests is this patch; added the correct expectations for fast/lists/positioned-count-crash.html
and fast/dom/shadow/shadow-and-list-elements.html

* dom/Node.cpp:
(WebCore::Node::pseudoAwarePreviousSibling):
(WebCore):
(WebCore::Node::pseudoAwareNextSibling):
(WebCore::Node::pseudoAwareFirstChild):
(WebCore::Node::pseudoAwareLastChild):
* dom/Node.h:
(Node):
* dom/NodeTraversal.cpp:
(WebCore):
(WebCore::NodeTraversal::previousIncludingPseudo):
(NodeTraversal):
(WebCore::NodeTraversal::nextIncludingPseudo):
(WebCore::NodeTraversal::nextIncludingPseudoSkippingChildren):
* dom/NodeTraversal.h:
(ElementTraversal):
(NodeTraversal):
(WebCore::ElementTraversal::previousIncludingPseudo):
(WebCore::ElementTraversal::nextIncludingPseudo):
(WebCore::ElementTraversal::nextIncludingPseudoSkippingChildren):
(WebCore::ElementTraversal::pseudoAwarePreviousSibling):
* html/HTMLLIElement.cpp:
(WebCore::HTMLLIElement::attach):
* html/HTMLOListElement.cpp:
(WebCore::HTMLOListElement::updateItemValues):
(WebCore::HTMLOListElement::recalculateItemCount):
* rendering/RenderCounter.cpp:
(WebCore::previousInPreOrder):
(WebCore::previousSiblingOrParent):
(WebCore::parentElement):
(WebCore::nextInPreOrder):
* rendering/RenderListItem.cpp:
(WebCore):
(WebCore::enclosingList):
(WebCore::RenderListItem::nextListItem):
(WebCore::previousListItem):
(WebCore::RenderListItem::calcValue):
(WebCore::RenderListItem::explicitValueChanged):
(WebCore::previousOrNextItem):
(WebCore::RenderListItem::updateListMarkerNumbers):
* rendering/RenderListItem.h:
(RenderListItem):

LayoutTests:

The fast/dom/shadow/shadow-and-list-elements-expected.html has changed because the list items
inside the shadow tree no longer see the root <ol> element.
The test fast/lists/positioned-count-crash.html has the correct rendering after changing
the list counting to be in DOM order.

* fast/dom/shadow/shadow-and-list-elements-expected.html:
* fast/lists/positioned-count-crash-expected.txt:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (148862 => 148863)


--- trunk/LayoutTests/ChangeLog	2013-04-22 07:57:46 UTC (rev 148862)
+++ trunk/LayoutTests/ChangeLog	2013-04-22 08:32:28 UTC (rev 148863)
@@ -1,3 +1,18 @@
+2013-04-22  Andrei Bucur  <abu...@adobe.com>
+
+        Use DOM ordering for list counts
+        https://bugs.webkit.org/show_bug.cgi?id=110352
+
+        Reviewed by Elliott Sprehn.
+
+        The fast/dom/shadow/shadow-and-list-elements-expected.html has changed because the list items
+        inside the shadow tree no longer see the root <ol> element.
+        The test fast/lists/positioned-count-crash.html has the correct rendering after changing
+        the list counting to be in DOM order.
+
+        * fast/dom/shadow/shadow-and-list-elements-expected.html:
+        * fast/lists/positioned-count-crash-expected.txt:
+
 2013-04-22  Seokju Kwon  <seokju.k...@gmail.com>
 
         [GTK] Cleanup TestExpectations to pass --lint-test-files

Modified: trunk/LayoutTests/fast/dom/shadow/shadow-and-list-elements-expected.html (148862 => 148863)


--- trunk/LayoutTests/fast/dom/shadow/shadow-and-list-elements-expected.html	2013-04-22 07:57:46 UTC (rev 148862)
+++ trunk/LayoutTests/fast/dom/shadow/shadow-and-list-elements-expected.html	2013-04-22 08:32:28 UTC (rev 148863)
@@ -22,18 +22,23 @@
 //   <li>C</li>
 // </ol>   
 var hostEquivalent = document.getElementById("hostEquivalent");
+var shadowListRoot = document.createElement("ol");
+shadowListRoot.style.paddingLeft = "0px";
+hostEquivalent.appendChild(shadowListRoot);
 
 var childX = document.createElement("li");
 childX.innerHTML = "X";
-hostEquivalent.appendChild(childX);
+childX.style.listStylePosition = "inside";
+shadowListRoot.appendChild(childX);
 
 var childUl = document.createElement("ul");
 childUl.innerHTML = "B";
-hostEquivalent.appendChild(childUl);
+shadowListRoot.appendChild(childUl);
 
 var childY = document.createElement("li");
 childY.innerHTML = "Y";
-hostEquivalent.appendChild(childY);
+childY.style.listStylePosition = "inside";
+shadowListRoot.appendChild(childY);
 </script>
 </body>
 </html>

Modified: trunk/LayoutTests/fast/lists/positioned-count-crash-expected.txt (148862 => 148863)


--- trunk/LayoutTests/fast/lists/positioned-count-crash-expected.txt	2013-04-22 07:57:46 UTC (rev 148862)
+++ trunk/LayoutTests/fast/lists/positioned-count-crash-expected.txt	2013-04-22 08:32:28 UTC (rev 148863)
@@ -2,5 +2,5 @@
 
 
 For manual test: If you see no crash and "II II", it means this test PASS.
-FAIL list marker should be II. Was I.
+PASS list marker is II.
 

Modified: trunk/Source/WebCore/ChangeLog (148862 => 148863)


--- trunk/Source/WebCore/ChangeLog	2013-04-22 07:57:46 UTC (rev 148862)
+++ trunk/Source/WebCore/ChangeLog	2013-04-22 08:32:28 UTC (rev 148863)
@@ -1,3 +1,77 @@
+2013-04-22  Andrei Bucur  <abu...@adobe.com>
+
+        Use DOM ordering for list counts
+        https://bugs.webkit.org/show_bug.cgi?id=110352
+
+        Reviewed by Elliott Sprehn.
+
+        Currently the list items ordering is made by traversing the render tree. This gives bad results for:
+        - list items flown inside regions because they are not rendered as descendants of their DOM list ancestors.
+        - list items matched to insertion points inside a shadow tree. The insertion point may be a child of a
+        list so the numbering gets broken.
+
+        To implement correct DOM ordering I've taken into account the fact that pseudo-elements can have display: list-item
+        so they should be included when traversing the DOM tree. I've added helper methods on the NodeTraversal
+        namespace that should allow easily traversing the tree including the pseudo-elements (e.g. firstChildWithPseudo
+        for an element with pseudo-before will return the before PseudoElement). Using these helper methods I've implemented
+        pre-order traversal methods aware of the pseudo-elements.
+        An effect of this change is how the list items inside shadow tree update their counting. With the patch, if there's
+        no <ol> or <ul> element on the ancestor chain of a <li> element to the shadow root, the list node will be considered the
+        first parent of the <li> or the shadow root if there is no ancestor.
+        The RenderListItem class now makes use of this new method of traversal, replacing the one based on the render tree.
+        To simplify the CSS counters implementation, I've changed the traversal functions inside RenderCounter to also make use
+        of this method. The CSS counters and the list items now have the same traversal algorithm.
+
+        In later patches I'll add tests that should cover the regions and shadow DOM use cases.
+        Tests bug for shadow DOM: https://bugs.webkit.org/show_bug.cgi?id=113193
+        Tests bug for regions: https://bugs.webkit.org/show_bug.cgi?id=103975
+
+        Tests: no new tests is this patch; added the correct expectations for fast/lists/positioned-count-crash.html
+        and fast/dom/shadow/shadow-and-list-elements.html
+
+        * dom/Node.cpp:
+        (WebCore::Node::pseudoAwarePreviousSibling):
+        (WebCore):
+        (WebCore::Node::pseudoAwareNextSibling):
+        (WebCore::Node::pseudoAwareFirstChild):
+        (WebCore::Node::pseudoAwareLastChild):
+        * dom/Node.h:
+        (Node):
+        * dom/NodeTraversal.cpp:
+        (WebCore):
+        (WebCore::NodeTraversal::previousIncludingPseudo):
+        (NodeTraversal):
+        (WebCore::NodeTraversal::nextIncludingPseudo):
+        (WebCore::NodeTraversal::nextIncludingPseudoSkippingChildren):
+        * dom/NodeTraversal.h:
+        (ElementTraversal):
+        (NodeTraversal):
+        (WebCore::ElementTraversal::previousIncludingPseudo):
+        (WebCore::ElementTraversal::nextIncludingPseudo):
+        (WebCore::ElementTraversal::nextIncludingPseudoSkippingChildren):
+        (WebCore::ElementTraversal::pseudoAwarePreviousSibling):
+        * html/HTMLLIElement.cpp:
+        (WebCore::HTMLLIElement::attach):
+        * html/HTMLOListElement.cpp:
+        (WebCore::HTMLOListElement::updateItemValues):
+        (WebCore::HTMLOListElement::recalculateItemCount):
+        * rendering/RenderCounter.cpp:
+        (WebCore::previousInPreOrder):
+        (WebCore::previousSiblingOrParent):
+        (WebCore::parentElement):
+        (WebCore::nextInPreOrder):
+        * rendering/RenderListItem.cpp:
+        (WebCore):
+        (WebCore::enclosingList):
+        (WebCore::RenderListItem::nextListItem):
+        (WebCore::previousListItem):
+        (WebCore::RenderListItem::calcValue):
+        (WebCore::RenderListItem::explicitValueChanged):
+        (WebCore::previousOrNextItem):
+        (WebCore::RenderListItem::updateListMarkerNumbers):
+        * rendering/RenderListItem.h:
+        (RenderListItem):
+
 2013-04-22  Alberto Garcia  <agar...@igalia.com>
 
         Fix multiple definition of ImageBuffer::platformLayer() in the BlackBerry port

Modified: trunk/Source/WebCore/dom/Node.cpp (148862 => 148863)


--- trunk/Source/WebCore/dom/Node.cpp	2013-04-22 07:57:46 UTC (rev 148862)
+++ trunk/Source/WebCore/dom/Node.cpp	2013-04-22 08:32:28 UTC (rev 148863)
@@ -1132,6 +1132,62 @@
 #endif
 }
 
+Node* Node::pseudoAwarePreviousSibling() const
+{
+    if (parentElement() && !previousSibling()) {
+        Element* parent = parentElement();
+        if (isAfterPseudoElement() && parent->lastChild())
+            return parent->lastChild();
+        if (!isBeforePseudoElement())
+            return parent->pseudoElement(BEFORE);
+    }
+    return previousSibling();
+}
+
+Node* Node::pseudoAwareNextSibling() const
+{
+    if (parentElement() && !nextSibling()) {
+        Element* parent = parentElement();
+        if (isBeforePseudoElement() && parent->firstChild())
+            return parent->firstChild();
+        if (!isAfterPseudoElement())
+            return parent->pseudoElement(AFTER);
+    }
+    return nextSibling();
+}
+
+Node* Node::pseudoAwareFirstChild() const
+{
+    if (isElementNode()) {
+        const Element* currentElement = toElement(this);
+        Node* first = currentElement->pseudoElement(BEFORE);
+        if (first)
+            return first;
+        first = currentElement->firstChild();
+        if (!first)
+            first = currentElement->pseudoElement(AFTER);
+        return first;
+    }
+
+    return firstChild();
+}
+
+Node* Node::pseudoAwareLastChild() const
+{
+    if (isElementNode()) {
+        const Element* currentElement = toElement(this);
+        Node* last = currentElement->pseudoElement(AFTER);
+        if (last)
+            return last;
+        last = currentElement->lastChild();
+        if (!last)
+            last = currentElement->pseudoElement(BEFORE);
+        return last;
+    }
+
+    return lastChild();
+}
+
 // FIXME: This code is used by editing.  Seems like it could move over there and not pollute Node.
 Node *Node::previousNodeConsideringAtomicNodes() const
 {

Modified: trunk/Source/WebCore/dom/Node.h (148862 => 148863)


--- trunk/Source/WebCore/dom/Node.h	2013-04-22 07:57:46 UTC (rev 148862)
+++ trunk/Source/WebCore/dom/Node.h	2013-04-22 08:32:28 UTC (rev 148863)
@@ -193,6 +193,10 @@
     Node* lastChild() const;
     bool hasAttributes() const;
     NamedNodeMap* attributes() const;
+    Node* pseudoAwareNextSibling() const;
+    Node* pseudoAwarePreviousSibling() const;
+    Node* pseudoAwareFirstChild() const;
+    Node* pseudoAwareLastChild() const;
 
     virtual KURL baseURI() const;
     

Modified: trunk/Source/WebCore/dom/NodeTraversal.cpp (148862 => 148863)


--- trunk/Source/WebCore/dom/NodeTraversal.cpp	2013-04-22 07:57:46 UTC (rev 148862)
+++ trunk/Source/WebCore/dom/NodeTraversal.cpp	2013-04-22 08:32:28 UTC (rev 148863)
@@ -26,10 +26,59 @@
 #include "NodeTraversal.h"
 
 #include "ContainerNode.h"
+#include "PseudoElement.h"
 
 namespace WebCore {
+
 namespace NodeTraversal {
 
+Node* previousIncludingPseudo(const Node* current, const Node* stayWithin)
+{
+    Node* previous;
+    if (current == stayWithin)
+        return 0;
+    if ((previous = current->pseudoAwarePreviousSibling())) {
+        while (previous->pseudoAwareLastChild())
+            previous = previous->pseudoAwareLastChild();
+        return previous;
+    }
+    return current->parentNode();
+}
+
+Node* nextIncludingPseudo(const Node* current, const Node* stayWithin)
+{
+    Node* next;
+    if ((next = current->pseudoAwareFirstChild()))
+        return next;
+    if (current == stayWithin)
+        return 0;
+    if ((next = current->pseudoAwareNextSibling()))
+        return next;
+    for (current = current->parentNode(); current; current = current->parentNode()) {
+        if (current == stayWithin)
+            return 0;
+        if ((next = current->pseudoAwareNextSibling()))
+            return next;
+    }
+    return 0;
+}
+
+Node* nextIncludingPseudoSkippingChildren(const Node* current, const Node* stayWithin)
+{
+    Node* next;
+    if (current == stayWithin)
+        return 0;
+    if ((next = current->pseudoAwareNextSibling()))
+        return next;
+    for (current = current->parentNode(); current; current = current->parentNode()) {
+        if (current == stayWithin)
+            return 0;
+        if ((next = current->pseudoAwareNextSibling()))
+            return next;
+    }
+    return 0;
+}
+
 Node* nextAncestorSibling(const Node* current)
 {
     ASSERT(!current->nextSibling());

Modified: trunk/Source/WebCore/dom/NodeTraversal.h (148862 => 148863)


--- trunk/Source/WebCore/dom/NodeTraversal.h	2013-04-22 07:57:46 UTC (rev 148862)
+++ trunk/Source/WebCore/dom/NodeTraversal.h	2013-04-22 08:32:28 UTC (rev 148863)
@@ -34,17 +34,27 @@
 // First element child of the node.
 Element* firstWithin(const Node*);
 Element* firstWithin(const ContainerNode*);
+
 // Pre-order traversal skipping non-element nodes.
 Element* next(const Node*);
 Element* next(const Node*, const Node* stayWithin);
 Element* next(const ContainerNode*);
 Element* next(const ContainerNode*, const Node* stayWithin);
+
 // Like next, but skips children.
 Element* nextSkippingChildren(const Node*);
 Element* nextSkippingChildren(const Node*, const Node* stayWithin);
 Element* nextSkippingChildren(const ContainerNode*);
 Element* nextSkippingChildren(const ContainerNode*, const Node* stayWithin);
 
+// Pre-order traversal including the pseudo-elements.
+Element* previousIncludingPseudo(const Node*, const Node* = 0);
+Element* nextIncludingPseudo(const Node*, const Node* = 0);
+Element* nextIncludingPseudoSkippingChildren(const Node*, const Node* = 0);
+
+// Utility function to traverse only the element and pseudo-element siblings of a node.
+Element* pseudoAwarePreviousSibling(const Node*);
+
 }
 
 namespace NodeTraversal {
@@ -77,6 +87,11 @@
 Node* previousPostOrder(const Node*, const Node* stayWithin = 0);
 Node* previousSkippingChildrenPostOrder(const Node*, const Node* stayWithin = 0);
 
+// Pre-order traversal including the pseudo-elements.
+Node* previousIncludingPseudo(const Node*, const Node* = 0);
+Node* nextIncludingPseudo(const Node*, const Node* = 0);
+Node* nextIncludingPseudoSkippingChildren(const Node*, const Node* = 0);
+
 }
 
 namespace ElementTraversal {
@@ -135,8 +150,41 @@
 }
 inline Element* nextSkippingChildren(const ContainerNode* current, const Node* stayWithin) { return traverseNextElementSkippingChildrenTemplate(current, stayWithin); }
 inline Element* nextSkippingChildren(const Node* current, const Node* stayWithin) { return traverseNextElementSkippingChildrenTemplate(current, stayWithin); }
+
+inline Element* previousIncludingPseudo(const Node* current, const Node* stayWithin)
+{
+    Node* node = NodeTraversal::previousIncludingPseudo(current, stayWithin);
+    while (node && !node->isElementNode())
+        node = NodeTraversal::previousIncludingPseudo(node, stayWithin);
+    return toElement(node);
 }
 
+inline Element* nextIncludingPseudo(const Node* current, const Node* stayWithin)
+{
+    Node* node = NodeTraversal::nextIncludingPseudo(current, stayWithin);
+    while (node && !node->isElementNode())
+        node = NodeTraversal::nextIncludingPseudo(node, stayWithin);
+    return toElement(node);
+}
+
+inline Element* nextIncludingPseudoSkippingChildren(const Node* current, const Node* stayWithin)
+{
+    Node* node = NodeTraversal::nextIncludingPseudoSkippingChildren(current, stayWithin);
+    while (node && !node->isElementNode())
+        node = NodeTraversal::nextIncludingPseudoSkippingChildren(node, stayWithin);
+    return toElement(node);
+}
+
+inline Element* pseudoAwarePreviousSibling(const Node* current)
+{
+    Node* node = current->pseudoAwarePreviousSibling();
+    while (node && !node->isElementNode())
+        node = node->pseudoAwarePreviousSibling();
+    return toElement(node);
+}
+
+}
+
 namespace NodeTraversal {
 
 Node* nextAncestorSibling(const Node*);

Modified: trunk/Source/WebCore/html/HTMLLIElement.cpp (148862 => 148863)


--- trunk/Source/WebCore/html/HTMLLIElement.cpp	2013-04-22 07:57:46 UTC (rev 148862)
+++ trunk/Source/WebCore/html/HTMLLIElement.cpp	2013-04-22 08:32:28 UTC (rev 148863)
@@ -92,23 +92,23 @@
     HTMLElement::attach();
 
     if (renderer() && renderer()->isListItem()) {
-        RenderListItem* render = toRenderListItem(renderer());
+        RenderListItem* listItemRenderer = toRenderListItem(renderer());
 
         // Find the enclosing list node.
-        Node* listNode = 0;
-        EventPathWalker walker(this);
+        Element* listNode = 0;
+        Element* current = this;
         while (!listNode) {
-            walker.moveToParent();
-            if (!walker.node())
+            current = current->parentElement();
+            if (!current)
                 break;
-            if (walker.node()->hasTagName(ulTag) || walker.node()->hasTagName(olTag))
-                listNode = walker.node();
+            if (current->hasTagName(ulTag) || current->hasTagName(olTag))
+                listNode = current;
         }
 
         // If we are not in a list, tell the renderer so it can position us inside.
         // We don't want to change our style to say "inside" since that would affect nested nodes.
         if (!listNode)
-            render->setNotInList(true);
+            listItemRenderer->setNotInList(true);
 
         parseValue(fastGetAttribute(valueAttr));
     }

Modified: trunk/Source/WebCore/html/HTMLOListElement.cpp (148862 => 148863)


--- trunk/Source/WebCore/html/HTMLOListElement.cpp	2013-04-22 07:57:46 UTC (rev 148862)
+++ trunk/Source/WebCore/html/HTMLOListElement.cpp	2013-04-22 08:32:28 UTC (rev 148863)
@@ -106,7 +106,7 @@
 
 void HTMLOListElement::updateItemValues()
 {
-    for (RenderListItem* listItem = RenderListItem::nextListItem(renderer()); listItem; listItem = RenderListItem::nextListItem(renderer(), listItem))
+    for (RenderListItem* listItem = RenderListItem::nextListItem(this); listItem; listItem = RenderListItem::nextListItem(this, listItem))
         listItem->updateValue();
 }
 
@@ -114,7 +114,7 @@
 {
     m_itemCount = 0;
 
-    for (RenderListItem* listItem = RenderListItem::nextListItem(renderer()); listItem; listItem = RenderListItem::nextListItem(renderer(), listItem))
+    for (RenderListItem* listItem = RenderListItem::nextListItem(this); listItem; listItem = RenderListItem::nextListItem(this, listItem))
         m_itemCount++;
 
     m_shouldRecalculateItemCount = false;

Modified: trunk/Source/WebCore/rendering/RenderCounter.cpp (148862 => 148863)


--- trunk/Source/WebCore/rendering/RenderCounter.cpp	2013-04-22 07:57:46 UTC (rev 148862)
+++ trunk/Source/WebCore/rendering/RenderCounter.cpp	2013-04-22 08:32:28 UTC (rev 148863)
@@ -57,94 +57,30 @@
 // including pseudo elements as defined in CSS 2.1.
 static RenderObject* previousInPreOrder(const RenderObject* object)
 {
-    Element* parent;
-    Element* sibling;
-    switch (object->style()->styleType()) {
-    case NOPSEUDO:
-        ASSERT(!object->isAnonymous());
-        parent = toElement(object->node());
-        sibling = parent->previousElementSibling();
-        parent = parent->parentElement();
-        break;
-    case BEFORE:
-        return object->generatingNode()->renderer(); // It is always the generating node's renderer
-    case AFTER:
-        parent = toElement(object->generatingNode());
-        sibling = parent->lastElementChild();
-        break;
-    default:
-        ASSERT_NOT_REACHED();
-        return 0;
-    }
-    while (sibling) {
-        if (RenderObject* renderer = sibling->renderer()) {
-            if (RenderObject* after = sibling->pseudoElementRenderer(AFTER))
-                return after;
-            parent = sibling;
-            sibling = sibling->lastElementChild();
-            if (!sibling) {
-                if (RenderObject* before = toElement(renderer->node())->pseudoElementRenderer(BEFORE))
-                    return before;
-                return renderer;
-            }
-        } else
-            sibling = sibling->previousElementSibling();
-    }
-    if (!parent)
-        return 0;
-    if (RenderObject* before = parent->pseudoElementRenderer(BEFORE))
-        return before;
-    return parent->renderer();
+    Element* self = toElement(object->node());
+    Element* previous = ElementTraversal::previousIncludingPseudo(self);
+    while (previous && !previous->renderer())
+        previous = ElementTraversal::previousIncludingPseudo(previous);
+    return previous ? previous->renderer() : 0;
 }
 
 // This function processes the renderer tree in the order of the DOM tree
 // including pseudo elements as defined in CSS 2.1.
 static RenderObject* previousSiblingOrParent(const RenderObject* object)
 {
-    Element* parent;
-    Element* sibling;
-    switch (object->style()->styleType()) {
-    case NOPSEUDO:
-        ASSERT(!object->isAnonymous());
-        parent = toElement(object->node());
-        sibling = parent->previousElementSibling();
-        parent = parent->parentElement();
-        break;
-    case BEFORE:
-        return object->generatingNode()->renderer(); // It is always the generating node's renderer
-    case AFTER:
-        parent = toElement(object->generatingNode());
-        sibling = parent->lastElementChild();
-        break;
-    default:
-        ASSERT_NOT_REACHED();
-        return 0;
-    }
-    while (sibling) {
-        if (RenderObject* renderer = sibling->renderer()) // This skips invisible nodes
-            return renderer;
-        sibling = sibling->previousElementSibling();
-    }
-    if (!parent)
-        return 0;
-    if (RenderObject* before = parent->pseudoElementRenderer(BEFORE))
-        return before;
-    return parent->renderer();
+    Element* self = toElement(object->node());
+    Element* previous = ElementTraversal::pseudoAwarePreviousSibling(self);
+    while (previous && !previous->renderer())
+        previous = ElementTraversal::pseudoAwarePreviousSibling(previous);
+    if (previous)
+        return previous->renderer();
+    previous = self->parentElement();
+    return previous ? previous->renderer() : 0;
 }
 
-static Element* parentElement(RenderObject* object)
+static inline Element* parentElement(RenderObject* object)
 {
-    switch (object->style()->styleType()) {
-    case NOPSEUDO:
-        ASSERT(!object->isAnonymous());
-        return toElement(object->node())->parentElement();
-    case BEFORE:
-    case AFTER:
-        return toElement(object->generatingNode());
-    default:
-        ASSERT_NOT_REACHED();
-        return 0;
-    }
+    return toElement(object->node())->parentElement();
 }
 
 static inline bool areRenderersElementsSiblings(RenderObject* first, RenderObject* second)
@@ -156,44 +92,11 @@
 // including pseudo elements as defined in CSS 2.1.
 static RenderObject* nextInPreOrder(const RenderObject* object, const Element* stayWithin, bool skipDescendants = false)
 {
-    Element* self;
-    Element* child;
-    self = toElement(object->generatingNode());
-    if (skipDescendants)
-        goto nextsibling;
-    switch (object->style()->styleType()) {
-    case NOPSEUDO:
-        ASSERT(!object->isAnonymous());
-        if (RenderObject* before = self->pseudoElementRenderer(BEFORE))
-            return before;
-        break;
-    case BEFORE:
-        break;
-    case AFTER:
-        goto nextsibling;
-    default:
-        ASSERT_NOT_REACHED();
-        return 0;
-    }
-    child = ElementTraversal::firstWithin(self);
-    while (true) {
-        while (child) {
-            if (RenderObject* renderer = child->renderer())
-                return renderer;
-            child = ElementTraversal::nextSkippingChildren(child, self);
-        }
-        if (RenderObject* after = self->pseudoElementRenderer(AFTER))
-            return after;
-nextsibling:
-        if (self == stayWithin)
-            return 0;
-        child = ElementTraversal::nextSkippingChildren(self);
-        self = self->parentElement();
-        if (!self) {
-            ASSERT(!child); // We can only reach this if we are searching beyond the root element
-            return 0; //  which cannot have siblings
-        }
-    }
+    Element* self = toElement(object->node());
+    Element* next = skipDescendants ? ElementTraversal::nextIncludingPseudoSkippingChildren(self, stayWithin) : ElementTraversal::nextIncludingPseudo(self, stayWithin);
+    while (next && !next->renderer())
+        next = skipDescendants ? ElementTraversal::nextIncludingPseudoSkippingChildren(next, stayWithin) : ElementTraversal::nextIncludingPseudo(next, stayWithin);
+    return next ? next->renderer() : 0;
 }
 
 static bool planCounter(RenderObject* object, const AtomicString& identifier, bool& isReset, int& value)

Modified: trunk/Source/WebCore/rendering/RenderListItem.cpp (148862 => 148863)


--- trunk/Source/WebCore/rendering/RenderListItem.cpp	2013-04-22 07:57:46 UTC (rev 148862)
+++ trunk/Source/WebCore/rendering/RenderListItem.cpp	2013-04-22 08:32:28 UTC (rev 148863)
@@ -26,6 +26,7 @@
 
 #include "HTMLNames.h"
 #include "HTMLOListElement.h"
+#include "NodeTraversal.h"
 #include "RenderListMarker.h"
 #include "RenderView.h"
 #include "StyleInheritedData.h"
@@ -96,18 +97,17 @@
     return (node->hasTagName(ulTag) || node->hasTagName(olTag));
 }
 
+// Returns the enclosing list with respect to the DOM order.
 static Node* enclosingList(const RenderListItem* listItem)
 {
+    Node* listItemNode = listItem->node();
     Node* firstNode = 0;
-
-    for (const RenderObject* renderer = listItem->parent(); renderer; renderer = renderer->parent()) {
-        Node* node = renderer->node();
-        if (node) {
-            if (isList(node))
-                return node;
-            if (!firstNode)
-                firstNode = node;
-        }
+    // We use parentNode because the enclosing list could be a ShadowRoot that's not Element.
+    for (Node* parent = listItemNode->parentNode(); parent; parent = parent->parentNode()) {
+        if (isList(parent))
+            return parent;
+        if (!firstNode)
+            firstNode = parent;
     }
 
     // If there's no actual <ul> or <ol> list element, then the first found
@@ -116,42 +116,51 @@
     return firstNode;
 }
 
-RenderListItem* RenderListItem::nextListItem(RenderObject* list, const RenderListItem* item)
+// Returns the next list item with respect to the DOM order.
+RenderListItem* RenderListItem::nextListItem(Node* listNode, const RenderListItem* item)
 {
-    if (!list)
+    if (!listNode)
         return 0;
 
-    RenderObject* renderer = item ? item->nextInPreOrder(list) : list->nextInPreOrder(list);
-    while (renderer) {
-        if (renderer->node() && isList(renderer->node())) {
+    Node* current = item ? item->node() : listNode;
+    current = ElementTraversal::nextIncludingPseudo(current, listNode);
+
+    while (current) {
+        if (isList(current)) {
             // We've found a nested, independent list: nothing to do here.
-            renderer = renderer->nextInPreOrderAfterChildren(list);
+            current = ElementTraversal::nextIncludingPseudoSkippingChildren(current, listNode);
             continue;
         }
 
-        if (renderer->isListItem())
+        RenderObject* renderer = current->renderer();
+        if (renderer && renderer->isListItem())
             return toRenderListItem(renderer);
 
-        renderer = renderer->nextInPreOrder(list);
+        // FIXME: Can this be optimized to skip the children of the elements without a renderer?
+        current = ElementTraversal::nextIncludingPseudo(current, listNode);
     }
+
     return 0;
 }
 
-static RenderListItem* previousListItem(RenderObject* list, const RenderListItem* item)
+// Returns the previous list item with respect to the DOM order.
+static RenderListItem* previousListItem(Node* listNode, const RenderListItem* item)
 {
-    for (RenderObject* renderer = item->previousInPreOrder(); renderer && renderer != list; renderer = renderer->previousInPreOrder()) {
-        if (!renderer->isListItem())
+    Node* current = item->node();
+    for (current = ElementTraversal::previousIncludingPseudo(current, listNode); current; current = ElementTraversal::previousIncludingPseudo(current, listNode)) {
+        RenderObject* renderer = current->renderer();
+        if (!renderer || (renderer && !renderer->isListItem()))
             continue;
         Node* otherList = enclosingList(toRenderListItem(renderer));
         // This item is part of our current list, so it's what we're looking for.
-        if (list->node() == otherList)
+        if (listNode == otherList)
             return toRenderListItem(renderer);
         // We found ourself inside another list; lets skip the rest of it.
-        // Use nextInPreOrder() here because the other list itself may actually
+        // Use nextIncludingPseudo() here because the other list itself may actually
         // be a list item itself. We need to examine it, so we do this to counteract
-        // the previousInPreOrder() that will be done by the loop.
+        // the previousIncludingPseudo() that will be done by the loop.
         if (otherList)
-            renderer = otherList->renderer()->nextInPreOrder();
+            current = ElementTraversal::nextIncludingPseudo(otherList);
     }
     return 0;
 }
@@ -162,7 +171,6 @@
         return m_explicitValue;
 
     Node* list = enclosingList(this);
-    RenderObject* listRenderer = list ? list->renderer() : 0;
     HTMLOListElement* oListElement = (list && list->hasTagName(olTag)) ? static_cast<HTMLOListElement*>(list) : 0;
     int valueStep = 1;
     if (oListElement && oListElement->isReversed())
@@ -170,7 +178,7 @@
 
     // FIXME: This recurses to a possible depth of the length of the list.
     // That's not good -- we need to change this to an iterative algorithm.
-    if (RenderListItem* previousItem = previousListItem(listRenderer, this))
+    if (RenderListItem* previousItem = previousListItem(list, this))
         return previousItem->value() + valueStep;
 
     if (oListElement)
@@ -427,10 +435,7 @@
     if (m_marker)
         m_marker->setNeedsLayoutAndPrefWidthsRecalc();
     Node* listNode = enclosingList(this);
-    RenderObject* listRenderer = 0;
-    if (listNode)
-        listRenderer = listNode->renderer();
-    for (RenderListItem* item = this; item; item = nextListItem(listRenderer, item))
+    for (RenderListItem* item = this; item; item = nextListItem(listNode, item))
         item->updateValue();
 }
 
@@ -457,7 +462,7 @@
     explicitValueChanged();
 }
 
-static RenderListItem* previousOrNextItem(bool isListReversed, RenderObject* list, RenderListItem* item)
+static RenderListItem* previousOrNextItem(bool isListReversed, Node* list, RenderListItem* item)
 {
     return isListReversed ? previousListItem(list, item) : RenderListItem::nextListItem(list, item);
 }
@@ -465,18 +470,18 @@
 void RenderListItem::updateListMarkerNumbers()
 {
     Node* listNode = enclosingList(this);
-    ASSERT(listNode && listNode->renderer());
-    if (!listNode || !listNode->renderer())
+    // The list node can be the shadow root which has no renderer.
+    ASSERT(listNode);
+    if (!listNode)
         return;
 
     bool isListReversed = false;
-    RenderObject* list = listNode->renderer();
     HTMLOListElement* oListElement = (listNode && listNode->hasTagName(olTag)) ? static_cast<HTMLOListElement*>(listNode) : 0;
     if (oListElement) {
         oListElement->itemCountChanged();
         isListReversed = oListElement->isReversed();
     }
-    for (RenderListItem* item = previousOrNextItem(isListReversed, list, this); item; item = previousOrNextItem(isListReversed, list, item)) {
+    for (RenderListItem* item = previousOrNextItem(isListReversed, listNode, this); item; item = previousOrNextItem(isListReversed, listNode, item)) {
         if (!item->m_isValueUpToDate) {
             // If an item has been marked for update before, we can safely
             // assume that all the following ones have too.

Modified: trunk/Source/WebCore/rendering/RenderListItem.h (148862 => 148863)


--- trunk/Source/WebCore/rendering/RenderListItem.h	2013-04-22 07:57:46 UTC (rev 148862)
+++ trunk/Source/WebCore/rendering/RenderListItem.h	2013-04-22 08:32:28 UTC (rev 148863)
@@ -49,7 +49,7 @@
 
     void updateListMarkerNumbers();
 
-    static RenderListItem* nextListItem(RenderObject* listRenderer, const RenderListItem* = 0);
+    static RenderListItem* nextListItem(Node*, const RenderListItem* = 0);
 
 private:
     virtual const char* renderName() const { return "RenderListItem"; }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to