diff options
author | darin <darin@bbb929c8-8fbe-4397-9dbb-9b2b20218538> | 2007-10-18 03:28:22 +0000 |
---|---|---|
committer | darin <darin@bbb929c8-8fbe-4397-9dbb-9b2b20218538> | 2007-10-18 03:28:22 +0000 |
commit | d71fc976392660d03b56c3d557a9ca3df5798fd1 (patch) | |
tree | 7a9cbf5cc3a1f63fb362873ac4143008159d099b /third_party | |
parent | 11b57c3ae1d53b683846c1ae7cddbdb99943d5e5 (diff) | |
download | chromium_src-d71fc976392660d03b56c3d557a9ca3df5798fd1.zip chromium_src-d71fc976392660d03b56c3d557a9ca3df5798fd1.tar.gz chromium_src-d71fc976392660d03b56c3d557a9ca3df5798fd1.tar.bz2 |
WebCore:
Reviewed by Darin, Adam, and Maciej.
http://bugs.webkit.org/show_bug.cgi?id=12988
First element (in document order) is not returned when other duplicate ID-ed elements were created first
Reset the element id cache when an id is added and there is a duplicate for that id.
* dom/Document.cpp:
(WebCore::Document::addElementById):
LayoutTests:
Reviewed by Darin, Adam, and Maciej.
Testcase for:
http://bugs.webkit.org/show_bug.cgi?id=12988
First element (in document order) is not returned when other duplicate ID-ed elements were created first
* fast/dom/duplicate-ids-document-order.html: Added.
git-svn-id: svn://svn.chromium.org/blink/trunk@26732 bbb929c8-8fbe-4397-9dbb-9b2b20218538
Diffstat (limited to 'third_party')
-rw-r--r-- | third_party/WebKit/LayoutTests/ChangeLog | 10 | ||||
-rw-r--r-- | third_party/WebKit/LayoutTests/fast/dom/duplicate-ids-document-order.html | 37 | ||||
-rw-r--r-- | third_party/WebKit/WebCore/ChangeLog | 12 | ||||
-rw-r--r-- | third_party/WebKit/WebCore/dom/Document.cpp | 28 | ||||
-rw-r--r-- | third_party/WebKit/WebCore/dom/Document.h | 3 |
5 files changed, 86 insertions, 4 deletions
diff --git a/third_party/WebKit/LayoutTests/ChangeLog b/third_party/WebKit/LayoutTests/ChangeLog index e19949e..606e2e0 100644 --- a/third_party/WebKit/LayoutTests/ChangeLog +++ b/third_party/WebKit/LayoutTests/ChangeLog @@ -1,3 +1,13 @@ +2007-10-17 Rob Buis <buis@kde.org> + + Reviewed by Darin, Adam, and Maciej. + + Testcase for: + http://bugs.webkit.org/show_bug.cgi?id=12988 + First element (in document order) is not returned when other duplicate ID-ed elements were created first + + * fast/dom/duplicate-ids-document-order.html: Added. + 2007-10-17 Alice Liu <alice.liu@apple.com> Rubber-stamped by Adele. diff --git a/third_party/WebKit/LayoutTests/fast/dom/duplicate-ids-document-order.html b/third_party/WebKit/LayoutTests/fast/dom/duplicate-ids-document-order.html new file mode 100644 index 0000000..5fdba94 --- /dev/null +++ b/third_party/WebKit/LayoutTests/fast/dom/duplicate-ids-document-order.html @@ -0,0 +1,37 @@ +<html> +<head> +<script> +function debug(str) { + text = document.createTextNode(str); + debugDiv = document.getElementById('debugDiv'); + div = document.createElement('div'); + div.appendChild(text); + debugDiv.appendChild(div); +} + +function runTest() { + if (window.layoutTestController) { + layoutTestController.dumpAsText(); + } + var div = document.getElementById("foo"); + for (var i = 0; i < 5; ++i) { + var span = document.createElement("span"); + span.appendChild(document.createTextNode(i)); + span.setAttribute("id", "bar"); + div.insertBefore(span, div.firstChild); + } + + if (document.getElementById("bar").innerHTML == 4) + debug("Success"); + else + debug("Failure"); +} +</script> +</head> +<body onload="runTest()"> +This tests that getElementById returns the first element in document order when there are multiple ids. Bug 12988. +<div style="display:none" id="foo">text</div> +<div id='debugDiv'> +</div> +</body> +</html> diff --git a/third_party/WebKit/WebCore/ChangeLog b/third_party/WebKit/WebCore/ChangeLog index 24ce1d7..7ac63c4 100644 --- a/third_party/WebKit/WebCore/ChangeLog +++ b/third_party/WebKit/WebCore/ChangeLog @@ -1,3 +1,15 @@ +2007-10-17 Rob Buis <buis@kde.org> + + Reviewed by Darin, Adam, and Maciej. + + http://bugs.webkit.org/show_bug.cgi?id=12988 + First element (in document order) is not returned when other duplicate ID-ed elements were created first + + Reset the element id cache when an id is added and there is a duplicate for that id. + + * dom/Document.cpp: + (WebCore::Document::addElementById): + 2007-10-17 Mark Rowe <mrowe@apple.com> Mac build fix. diff --git a/third_party/WebKit/WebCore/dom/Document.cpp b/third_party/WebKit/WebCore/dom/Document.cpp index fe58fe12..6e43b64 100644 --- a/third_party/WebKit/WebCore/dom/Document.cpp +++ b/third_party/WebKit/WebCore/dom/Document.cpp @@ -711,8 +711,9 @@ Element *Document::getElementById(const AtomicString& elementId) const Element *element = m_elementsById.get(elementId.impl()); if (element) return element; - + if (m_duplicateIds.contains(elementId.impl())) { + // We know there's at least one node with this id, but we don't know what the first one is. for (Node *n = traverseNextNode(); n != 0; n = n->traverseNextNode()) { if (n->isElementNode()) { element = static_cast<Element*>(n); @@ -723,6 +724,7 @@ Element *Document::getElementById(const AtomicString& elementId) const } } } + ASSERT_NOT_REACHED(); } return 0; } @@ -817,10 +819,28 @@ Element* Document::elementFromPoint(int x, int y) const void Document::addElementById(const AtomicString& elementId, Element* element) { - if (!m_elementsById.contains(elementId.impl())) - m_elementsById.set(elementId.impl(), element); - else + typedef HashMap<AtomicStringImpl*, Element*>::iterator iterator; + if (!m_duplicateIds.contains(elementId.impl())) { + // Fast path. The ID is not already in m_duplicateIds, so we assume that it's + // also not already in m_elementsById and do an add. If that add succeeds, we're done. + pair<iterator, bool> addResult = m_elementsById.add(elementId.impl(), element); + if (addResult.second) + return; + // The add failed, so this ID was already cached in m_elementsById. + // There are multiple elements with this ID. Remove the m_elementsById + // cache for this ID so getElementById searches for it next time it is called. + m_elementsById.remove(addResult.first); m_duplicateIds.add(elementId.impl()); + } else { + // There are multiple elements with this ID. If it exists, remove the m_elementsById + // cache for this ID so getElementById searches for it next time it is called. + iterator cachedItem = m_elementsById.find(elementId.impl()); + if (cachedItem != m_elementsById.end()) { + m_elementsById.remove(cachedItem); + m_duplicateIds.add(elementId.impl()); + } + } + m_duplicateIds.add(elementId.impl()); } void Document::removeElementById(const AtomicString& elementId, Element* element) diff --git a/third_party/WebKit/WebCore/dom/Document.h b/third_party/WebKit/WebCore/dom/Document.h index 1df36e7..b9c476a 100644 --- a/third_party/WebKit/WebCore/dom/Document.h +++ b/third_party/WebKit/WebCore/dom/Document.h @@ -880,6 +880,9 @@ private: RefPtr<TextResourceDecoder> m_decoder; + // We maintain the invariant that m_duplicateIds is the count of all elements with a given ID + // excluding the one referenced in m_elementsById, if any. This means it one less than the total count + // when the first node with a given ID is cached, otherwise the same as the total count. mutable HashMap<AtomicStringImpl*, Element*> m_elementsById; mutable HashCountedSet<AtomicStringImpl*> m_duplicateIds; |