Skip to content

GitLab

T
tangled
Commit 9ba82696 authored by ch.dumez@samsung.com's avatar ch.dumez@samsung.com
Browse files
Options

Do not fire a DOMSubtreeModified event when Attr.value attribute is set 

Do not fire a DOMSubtreeModified event when Attr.value attribute is set. This
event is deprecated and both Firefox 31 and IE 11 do not fire it in this case.

Firing a DOMSubtreeModified event is problematic because it is synchronous and
it can lead to calling Attr::setValueInternal() recursively if the JS callback
sets Attr.value again. This can lead to serious problems as setValueInternal()
calls:
1. Element::willModifyAttribute(oldValue, newValue)
2. Attr::setValue(newValue);
3. Element::didModifyAttribute(newValue)

Due to recursivity, we can end up with the following call stack:
1. Element::willModifyAttribute("old-id", "new-id")
2. Attr::setValue("new-id");
3. Element::willModifyAttribute("new-id", "id-from-callback")
4. Attr::setValue("id-from-callback");
5. Element::didModifyAttribute("id-from-callback")
6. Element::didModifyAttribute("new-id")

After this, the Element's id is "new-id" because the id attribute is updated
in Element::didModifyAttribute(). However, the id in the DocumentOrderedMap
is "id-from-callback" because the id DocumentOrderedMap is updated in
Element::willModifyAttribute(). This mismatch between the DocumentOrderedMap
and the actual Element id can cause getElementById() to return incorrect
result. Even worse, it can result in a use-after-free (as in Bug 402255)
because the DocumentOrderedMap does not hold a reference to the StringImpl
used as key, assuming the StringImpl will be kept alive as it is used as 'id'
for an Element.

BUG=402255
TEST=fast/dom/Attr/set-attr-value-no-DOMSubtreeModified.html

Review URL: https://codereview.chromium.org/452093003

git-svn-id: svn://svn.chromium.org/blink/trunk@180380 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent c9688dd0
Hide whitespace changes
Inline Side-by-side
Showing with 32 additions and 14 deletions
third_party/WebKit/LayoutTests/fast/dom/Attr/set-attr-value-no-DOMSubtreeModified-expected.txt 0 → 100644
View file @ 9ba82696
Tests that setting Attr.value does not fire a DOMSubtreeModified event.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS wasDOMSubtreeModifiedFired is false
PASS wasDOMSubtreeModifiedFired is false
PASS successfullyParsed is true
TEST COMPLETE
third_party/WebKit/LayoutTests/fast/dom/Attr/set-attr-value-no-DOMSubtreeModified.html 0 → 100644
View file @ 9ba82696
<!DOCTYPE html>
<script src="../../../resources/js-test.js"></script>
<div id="testDiv"></div>
<script>
description("Tests that setting Attr.value does not fire a DOMSubtreeModified event.");
var testDiv = document.getElementById("testDiv");
var wasDOMSubtreeModifiedFired = false;
testDiv.attributes[0].addEventListener("DOMSubtreeModified", function() {
wasDOMSubtreeModifiedFired = true;
}, false);
shouldBeFalse("wasDOMSubtreeModifiedFired");
testDiv.attributes[0].value = "testDiv2";
shouldBeFalse("wasDOMSubtreeModifiedFired");
</script>
third_party/WebKit/LayoutTests/fast/dom/attribute-change-on-mutate.html deleted 100644 → 0
View file @ c9688dd0
<script>
if (window.testRunner)
testRunner.dumpAsText();
window.onload = function()
{
attr = document.createAttribute("attr");
attr.appendChild(document.createTextNode("FAIL"));
attr.addEventListener("DOMSubtreeModified", function () { document.body.innerHTML = this.value }, false);
attr.value = "PASS: Mutation events delayed during Attr modification.";
}
</script>
third_party/WebKit/Source/core/dom/ContainerNode.cpp
View file @ 9ba82696
...@@ -670,7 +670,11 @@ void ContainerNode::removeChildren() ...@@ -670,7 +670,11 @@ void ContainerNode::removeChildren()
childrenChanged(change); childrenChanged(change);
} }
dispatchSubtreeModifiedEvent(); // We don't fire the DOMSubtreeModified event for Attr Nodes. This matches the behavior
// of IE and Firefox. This event is fired synchronously and is a source of trouble for
// attributes as the JS callback could alter the attributes and leave us in a bad state.
if (!isAttributeNode())
dispatchSubtreeModifiedEvent();
} }
PassRefPtrWillBeRawPtr<Node> ContainerNode::appendChild(PassRefPtrWillBeRawPtr<Node> newChild, ExceptionState& exceptionState) PassRefPtrWillBeRawPtr<Node> ContainerNode::appendChild(PassRefPtrWillBeRawPtr<Node> newChild, ExceptionState& exceptionState)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment