From d71fc976392660d03b56c3d557a9ca3df5798fd1 Mon Sep 17 00:00:00 2001 From: darin Date: Thu, 18 Oct 2007 03:28:22 +0000 Subject: 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 --- third_party/WebKit/LayoutTests/ChangeLog | 10 ++++++ .../fast/dom/duplicate-ids-document-order.html | 37 ++++++++++++++++++++++ third_party/WebKit/WebCore/ChangeLog | 12 +++++++ third_party/WebKit/WebCore/dom/Document.cpp | 28 +++++++++++++--- third_party/WebKit/WebCore/dom/Document.h | 3 ++ 5 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 third_party/WebKit/LayoutTests/fast/dom/duplicate-ids-document-order.html (limited to 'third_party') 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 + + 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 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 @@ + + + + + +This tests that getElementById returns the first element in document order when there are multiple ids. Bug 12988. + +

+ + 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 + + 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 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(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::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 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 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 m_elementsById; mutable HashCountedSet m_duplicateIds; -- cgit v1.1