[Webkit-unassigned] [Bug 105324] Use ElementTraversal in LiveNodeListBase
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Dec 18 11:47:30 PST 2012
https://bugs.webkit.org/show_bug.cgi?id=105324
--- Comment #4 from Ryosuke Niwa <rniwa at webkit.org> 2012-12-18 11:49:46 PST ---
(From update of attachment 179980)
View in context: https://bugs.webkit.org/attachment.cgi?id=179980&action=review
> Source/WebCore/ChangeLog:9
> +
Leading whitespace.
> Source/WebCore/ChangeLog:11
> +
Ditto.
> Source/WebCore/ChangeLog:13
> +
Ditto.
> Source/WebCore/ChangeLog:21
> +
Ditto for the rest of the change log entry.
> Source/WebCore/ChangeLog:27
> + Root is always ContainerNode if the list has anything in it. Traversal functions are slightly more
Root is always ContainerNode. period. We only instantiate LiveNodeListBase on Document or Element.
> Source/WebCore/dom/LiveNodeList.cpp:59
> + if (!rootNode->isContainerNode())
> + return 0;
I don't think this can ever occur. m_ownerNode is always element or document, and root node can only be m_ownerNode or its ancestor.
> Source/WebCore/dom/TagNodeList.h:45
> - return adoptRef(new TagNodeList(rootNode, starAtom, localName));
> + return adoptRef(new TagNodeList(rootNode, TagNodeListType, starAtom, localName));
You can just pass in type here.
> Source/WebCore/html/HTMLCollection.cpp:382
> + if (type() == HTMLTagNodeListType)
> + return firstMatchingElement(static_cast<const HTMLTagNodeList*>(this), root);
> + if (type() == ClassNodeListType)
> + return firstMatchingElement(static_cast<const ClassNodeList*>(this), root);
So the reason you're cherry-picking these is because they're most frequently used?
Why don't we just special-case ChildNodeListType and then handle the rest in isAcceptableElement instead? There isn't really a point in differentiating NodeList and HTMLCollection here.
> Source/WebCore/html/HTMLCollection.cpp:470
> + ContainerNode* root = rootContainerNode();
> + if (!root) {
> + setLengthCache(0);
> + return 0;
> + }
It's really strange that rootContainerNode could ever return 0. Do you recall when that happened?
We probably need to fix that bug first.
> Source/WebCore/html/HTMLCollection.cpp:522
> + currentItem = traverseForwardToOffset(offset, currentItem, currentOffset, root);
> + else
> + currentItem = static_cast<const HTMLCollection*>(this)->traverseForwardToOffset(offset, toElement(currentItem), currentOffset, offsetInArray, root);
Note that we know currentOffset != offset at this point.
> Source/WebCore/html/HTMLCollection.cpp:584
> + offsetInArray = m_cachedElementsArrayOffset;
> + while (currentOffset != offset && (currentElement = virtualItemAfter(offsetInArray, currentElement)))
> + ++currentOffset;
This doesn't seem right virtualItemAfter needs to update m_cachedElementsArrayOffset. You probably need to enable microdata in order to test this change.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list