WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
[patch]
Patch
bug-74448-20111213142605.patch (text/plain), 20.24 KB, created by
Adam Klein
on 2011-12-13 14:26:06 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Adam Klein
Created:
2011-12-13 14:26:06 PST
Size:
20.24 KB
patch
obsolete
>Subversion Revision: 102705 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 192aeddc7e90dbaf4186698e121af20c38bb2441..af2b19785019099bee6639679547efd983dd3448 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,5 +1,31 @@ > 2011-12-13 Adam Klein <adamk@chromium.org> > >+ Broaden support for mutation observation of attributes >+ https://bugs.webkit.org/show_bug.cgi?id=74448 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ The previously-landed MutationObserver support for attributes was incomplete: >+ it didn't support mutations related to Attr nodes (methods on Attrs, >+ setAttributeNode/removeAttributeNode on Element, or methods on NamedNodeMap). >+ >+ This patch adds full support of mutation observation for all these cases, and adds >+ test cases for all these situations. >+ >+ * dom/Attr.cpp: >+ (WebCore::Attr::setValue): Enqueue a mutation record when Attr.value is set from JS. >+ (WebCore::Attr::childrenChanged): Enqueue a mutation record when an Attr's value changes to due additions/removals of Text children. >+ * dom/Element.cpp: >+ (WebCore::Element::enqueueAttributesMutationRecordIfRequested): Previously a static, expose as part of Element's interface to allow it to be re-used by NamedNodeMap and Attr. >+ (WebCore::Element::removeAttribute): Remove enqueue call now handled by NamedNodeMap. >+ (WebCore::Element::setAttributeInternal): Fixup call of enqueueAttributesMutationRecordIfRequested. >+ * dom/Element.h: >+ * dom/NamedNodeMap.cpp: >+ (WebCore::NamedNodeMap::setNamedItem): Enqueue a mutation record when an attribute is changed via Element.attributes.setNamedItem from JS. >+ (WebCore::NamedNodeMap::removeNamedItem): Enqueue a mutation record when an attribute is removed, either via Element.attributes.removeNamedItem or Element.removeAttribute. >+ >+2011-12-13 Adam Klein <adamk@chromium.org> >+ > Update variable names in NamedNodeMap methods to match WebKit style > https://bugs.webkit.org/show_bug.cgi?id=74437 > >diff --git a/Source/WebCore/dom/Attr.cpp b/Source/WebCore/dom/Attr.cpp >index ae98fe352cdbc2add27496501cb4f943d391f174..db69eb34670254bd19a371e702e99ef4b04669fa 100644 >--- a/Source/WebCore/dom/Attr.cpp >+++ b/Source/WebCore/dom/Attr.cpp >@@ -130,6 +130,11 @@ void Attr::setValue(const AtomicString& value) > > void Attr::setValue(const AtomicString& value, ExceptionCode&) > { >+#if ENABLE(MUTATION_OBSERVERS) >+ if (m_element) >+ m_element->enqueueAttributesMutationRecordIfRequested(m_attribute->name(), m_attribute->value()); >+#endif >+ > if (m_element && m_element->isIdAttributeName(m_attribute->name())) > m_element->updateId(m_element->getIdAttribute(), value); > >@@ -167,7 +172,12 @@ void Attr::childrenChanged(bool changedByParser, Node* beforeChange, Node* after > { > if (m_ignoreChildrenChanged > 0) > return; >- >+ >+#if ENABLE(MUTATION_OBSERVERS) >+ if (m_element) >+ m_element->enqueueAttributesMutationRecordIfRequested(m_attribute->name(), m_attribute->value()); >+#endif >+ > Node::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta); > > invalidateNodeListsCacheAfterAttributeChanged(); >diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp >index 5ddf5897104393c8515d24f4994a677ea476bea2..69779154f96bb8fec3cd35854d901e475ee44791 100644 >--- a/Source/WebCore/dom/Element.cpp >+++ b/Source/WebCore/dom/Element.cpp >@@ -181,10 +181,14 @@ void Element::copyNonAttributeProperties(const Element*) > } > > #if ENABLE(MUTATION_OBSERVERS) >-static void enqueueAttributesMutationRecord(Element* target, const QualifiedName& attributeName, const AtomicString& oldValue) >+void Element::enqueueAttributesMutationRecordIfRequested(const QualifiedName& attributeName, const AtomicString& oldValue) > { >- OwnPtr<MutationObserverInterestGroup> mutationRecipients = MutationObserverInterestGroup::createForAttributesMutation(target, attributeName); >- mutationRecipients->enqueueMutationRecord(MutationRecord::createAttributes(target, attributeName, oldValue)); >+ if (isSynchronizingStyleAttribute()) >+ return; >+ OwnPtr<MutationObserverInterestGroup> mutationRecipients = MutationObserverInterestGroup::createForAttributesMutation(this, attributeName); >+ if (mutationRecipients->isEmpty()) >+ return; >+ mutationRecipients->enqueueMutationRecord(MutationRecord::createAttributes(this, attributeName, oldValue)); > } > #endif > >@@ -195,10 +199,6 @@ void Element::removeAttribute(const QualifiedName& name, ExceptionCode& ec) > RefPtr<Node> attrNode = m_attributeMap->removeNamedItem(name, ec); > if (ec == NOT_FOUND_ERR) > ec = 0; >-#if ENABLE(MUTATION_OBSERVERS) >- else >- enqueueAttributesMutationRecord(this, name, attrNode->nodeValue()); >-#endif > } > } > >@@ -656,14 +656,12 @@ void Element::setAttributeInternal(Attribute* old, const QualifiedName& name, co > InspectorInstrumentation::willModifyDOMAttr(document(), this); > #endif > >- document()->incDOMTreeVersion(); >- > #if ENABLE(MUTATION_OBSERVERS) >- // The call to attributeChanged below may dispatch DOMSubtreeModified, so it's important to enqueue a MutationRecord now. >- if (!isSynchronizingStyleAttribute()) >- enqueueAttributesMutationRecord(this, name, old ? old->value() : nullAtom); >+ enqueueAttributesMutationRecordIfRequested(name, old ? old->value() : nullAtom); > #endif > >+ document()->incDOMTreeVersion(); >+ > if (isIdAttributeName(name)) > updateId(old ? old->value() : nullAtom, value); > >@@ -676,6 +674,7 @@ void Element::setAttributeInternal(Attribute* old, const QualifiedName& name, co > attrNode->setValue(value); > else > old->setValue(value); >+ > attributeChanged(old); > } > >@@ -1523,10 +1522,6 @@ void Element::removeAttribute(const String& name, ExceptionCode& ec) > RefPtr<Node> attrNode = m_attributeMap->removeNamedItem(localName, ec); > if (ec == NOT_FOUND_ERR) > ec = 0; >-#if ENABLE(MUTATION_OBSERVERS) >- else >- enqueueAttributesMutationRecord(this, QualifiedName(nullAtom, localName, nullAtom), attrNode->nodeValue()); >-#endif > } > > InspectorInstrumentation::didRemoveDOMAttr(document(), this, name); >diff --git a/Source/WebCore/dom/Element.h b/Source/WebCore/dom/Element.h >index 7a728633181134140a919dd552c46f8e768b6873..1d1728bfb109cb8ed498438abedefe8913610750 100644 >--- a/Source/WebCore/dom/Element.h >+++ b/Source/WebCore/dom/Element.h >@@ -364,6 +364,10 @@ public: > > PassRefPtr<RenderStyle> styleForRenderer(); > >+#if ENABLE(MUTATION_OBSERVERS) >+ void enqueueAttributesMutationRecordIfRequested(const QualifiedName&, const AtomicString& oldValue); >+#endif >+ > protected: > Element(const QualifiedName& tagName, Document* document, ConstructionType type) > : ContainerNode(document, type) >diff --git a/Source/WebCore/dom/NamedNodeMap.cpp b/Source/WebCore/dom/NamedNodeMap.cpp >index dc535014cfc117e966b617a6ada09677145a615e..960410d39f49372935551c50b043b68aee05f999 100644 >--- a/Source/WebCore/dom/NamedNodeMap.cpp >+++ b/Source/WebCore/dom/NamedNodeMap.cpp >@@ -119,6 +119,10 @@ PassRefPtr<Node> NamedNodeMap::setNamedItem(Node* node, ExceptionCode& ec) > return 0; > } > >+#if ENABLE(MUTATION_OBSERVERS) >+ m_element->enqueueAttributesMutationRecordIfRequested(attribute->name(), oldAttribute ? oldAttribute->value() : nullAtom); >+#endif >+ > if (attr->isId()) > m_element->updateId(oldAttribute ? oldAttribute->value() : nullAtom, attribute->value()); > >@@ -149,6 +153,11 @@ PassRefPtr<Node> NamedNodeMap::removeNamedItem(const QualifiedName& name, Except > return 0; > } > >+#if ENABLE(MUTATION_OBSERVERS) >+ if (m_element) >+ m_element->enqueueAttributesMutationRecordIfRequested(attribute->name(), attribute->value()); >+#endif >+ > RefPtr<Attr> attr = attribute->createAttrIfNeeded(m_element); > > if (attr->isId()) >@@ -242,7 +251,7 @@ void NamedNodeMap::setAttributes(const NamedNodeMap& other) > void NamedNodeMap::addAttribute(PassRefPtr<Attribute> prpAttribute) > { > RefPtr<Attribute> attribute = prpAttribute; >- >+ > // Add the attribute to the list > m_attributes.append(attribute); > >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index 7770f8f0402d1b620e146c4fd838b59858b5b638..664b27b06ddf7ada1967c385872c8964d6d6f1fc 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,15 @@ >+2011-12-13 Adam Klein <adamk@chromium.org> >+ >+ Broaden support for mutation observation of attributes >+ https://bugs.webkit.org/show_bug.cgi?id=74448 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Add tests covering attribute mutation via Attr nodes. >+ >+ * fast/mutation/observe-attributes-expected.txt: >+ * fast/mutation/observe-attributes.html: >+ > 2011-12-13 Vsevolod Vlasov <vsevik@chromium.org> > > Web Inspector: Network item view does not correctly decode "+" in request parameters. >diff --git a/LayoutTests/fast/mutation/observe-attributes-expected.txt b/LayoutTests/fast/mutation/observe-attributes-expected.txt >index 7fffc5f3545e0df567a4c8d5bc331fd9619e6e8a..83eb7bae2d320a823dbf50fb321e8f4e40a7d1a3 100644 >--- a/LayoutTests/fast/mutation/observe-attributes-expected.txt >+++ b/LayoutTests/fast/mutation/observe-attributes-expected.txt >@@ -90,13 +90,16 @@ PASS mutations[0].attributeName is "foo" > PASS mutations[0].oldValue is "bar" > > Testing setting an attribute via reflected IDL attribute. >-PASS mutations.length is 2 >+PASS mutations.length is 3 > PASS mutations[0].type is "attributes" > PASS mutations[0].attributeName is "id" > PASS mutations[0].oldValue is null > PASS mutations[1].type is "attributes" > PASS mutations[1].attributeName is "id" > PASS mutations[1].oldValue is "foo" >+PASS mutations[2].type is "attributes" >+PASS mutations[2].attributeName is "id" >+PASS mutations[2].oldValue is "bar" > > Testing that attributeFilter works as expected and ignores case with HTML elements. > ...only foo, bar & boom should be received. >@@ -189,6 +192,53 @@ PASS mutations is null > Testing that a no-op style property mutation does not create Mutation Records. > PASS mutations is null > >+Test that mutating an attribute through an attr node delivers mutation records >+PASS mutations.length is 1 >+PASS mutations[0].target is div >+PASS mutations[0].type is "attributes" >+PASS mutations[0].attributeName is "data-test" >+PASS mutations[0].oldValue is "foo" >+ >+Test that mutating an attribute by attaching a child to an attr node delivers mutation records >+PASS mutations.length is 1 >+PASS mutations[0].target is div >+PASS mutations[0].type is "attributes" >+PASS mutations[0].attributeName is "data-test" >+PASS mutations[0].oldValue is "foo" >+ >+Test that mutating via setAttributeNode delivers mutation records >+PASS mutations.length is 3 >+PASS mutations[0].target is div >+PASS mutations[0].type is "attributes" >+PASS mutations[0].attributeName is "data-test" >+PASS mutations[0].oldValue is "foo" >+PASS mutations[1].target is div >+PASS mutations[1].type is "attributes" >+PASS mutations[1].attributeName is "data-other" >+PASS mutations[1].oldValue is null >+PASS mutations[2].target is div >+PASS mutations[2].type is "attributes" >+PASS mutations[2].attributeName is "id" >+PASS mutations[2].oldValue is "myId" >+ >+Test that setAttribute on an attribute with an existing Attr delivers mutation records >+PASS mutations.length is 1 >+PASS mutations[0].target is div >+PASS mutations[0].type is "attributes" >+PASS mutations[0].attributeName is "data-test" >+PASS mutations[0].oldValue is "foo" >+ >+Test that setNamedItem and removeNamedItem deliver mutation records >+PASS mutations.length is 2 >+PASS mutations[0].target is div >+PASS mutations[0].type is "attributes" >+PASS mutations[0].attributeName is "data-test" >+PASS mutations[0].oldValue is "foo" >+PASS mutations[1].target is div >+PASS mutations[1].type is "attributes" >+PASS mutations[1].attributeName is "data-test" >+PASS mutations[1].oldValue is "bar" >+ > PASS successfullyParsed is true > > TEST COMPLETE >diff --git a/LayoutTests/fast/mutation/observe-attributes.html b/LayoutTests/fast/mutation/observe-attributes.html >index 0ee36ff70bd59e6e44816776f96993e4efa2ff45..a47ef6ac73d7255319402b3d16348c5eecc08ba2 100644 >--- a/LayoutTests/fast/mutation/observe-attributes.html >+++ b/LayoutTests/fast/mutation/observe-attributes.html >@@ -1,17 +1,11 @@ > <!DOCTYPE html> >-<html> >-<head> >-<meta charset="utf-8"> > <script src="../js/resources/js-test-pre.js"></script> >-</head> >-<body> >-<p id=description></p> >-<div id="console"></div> > <script> > > window.jsTestIsAsync = true; > var mutations, mutations2, mutationsWithOldValue; > var calls; >+var div; > > function testBasic() { > var div; >@@ -429,17 +423,21 @@ function testIDLAttribute() { > observer.observe(div, { attributes: true, attributeOldValue: true }); > div.id = 'foo'; > div.id = 'bar'; >+ div.id = null; > setTimeout(finish, 0); > } > > function finish() { >- shouldBe('mutations.length', '2'); >+ shouldBe('mutations.length', '3'); > shouldBe('mutations[0].type', '"attributes"'); > shouldBe('mutations[0].attributeName', '"id"'); > shouldBe('mutations[0].oldValue', 'null'); > shouldBe('mutations[1].type', '"attributes"'); > shouldBe('mutations[1].attributeName', '"id"'); > shouldBe('mutations[1].oldValue', '"foo"'); >+ shouldBe('mutations[2].type', '"attributes"'); >+ shouldBe('mutations[2].attributeName', '"id"'); >+ shouldBe('mutations[2].oldValue', '"bar"'); > observer.disconnect(); > debug(''); > runNextTest(); >@@ -786,6 +784,200 @@ function testStyleAttributePropertyAccessIgnoreNoop() { > start(); > } > >+function testMutateThroughAttrNodeValue() { >+ var observer; >+ >+ function start() { >+ debug('Test that mutating an attribute through an attr node delivers mutation records'); >+ >+ mutations = null; >+ observer = new WebKitMutationObserver(function(mutations) { >+ window.mutations = mutations; >+ }); >+ >+ div = document.createElement('div'); >+ div.setAttribute('data-test', 'foo'); >+ observer.observe(div, { attributes: true, attributeOldValue: true }); >+ div.attributes['data-test'].value = 'bar'; >+ >+ setTimeout(finish, 0); >+ } >+ >+ function finish() { >+ shouldBe('mutations.length', '1'); >+ shouldBe('mutations[0].target', 'div'); >+ shouldBe('mutations[0].type', '"attributes"'); >+ shouldBe('mutations[0].attributeName', '"data-test"'); >+ shouldBe('mutations[0].oldValue', '"foo"'); >+ >+ observer.disconnect(); >+ debug(''); >+ runNextTest(); >+ } >+ >+ start(); >+} >+ >+function testMutateThroughAttrNodeChild() { >+ var observer; >+ >+ function start() { >+ debug('Test that mutating an attribute by attaching a child to an attr node delivers mutation records'); >+ >+ mutations = null; >+ observer = new WebKitMutationObserver(function(mutations) { >+ window.mutations = mutations; >+ }); >+ >+ div = document.createElement('div'); >+ div.setAttribute('data-test', 'foo'); >+ observer.observe(div, { attributes: true, attributeOldValue: true }); >+ div.attributes['data-test'].appendChild(document.createTextNode('bar')); >+ >+ setTimeout(finish, 0); >+ } >+ >+ function finish() { >+ shouldBe('mutations.length', '1'); >+ shouldBe('mutations[0].target', 'div'); >+ shouldBe('mutations[0].type', '"attributes"'); >+ shouldBe('mutations[0].attributeName', '"data-test"'); >+ shouldBe('mutations[0].oldValue', '"foo"'); >+ >+ observer.disconnect(); >+ debug(''); >+ runNextTest(); >+ } >+ >+ start(); >+} >+ >+function testSetAndRemoveAttributeNode() { >+ var observer; >+ >+ function start() { >+ debug('Test that mutating via setAttributeNode delivers mutation records'); >+ >+ mutations = null; >+ observer = new WebKitMutationObserver(function(mutations) { >+ window.mutations = mutations; >+ }); >+ >+ div = document.createElement('div'); >+ div.id = 'myId'; >+ div.setAttribute('data-test', 'foo'); >+ observer.observe(div, { attributes: true, attributeOldValue: true }); >+ var attr = document.createAttribute('data-test'); >+ attr.value = 'bar'; >+ div.setAttributeNode(attr); >+ attr = document.createAttribute('data-other'); >+ attr.value = 'baz'; >+ div.setAttributeNode(attr); >+ div.removeAttributeNode(div.attributes['id']); >+ >+ setTimeout(finish, 0); >+ } >+ >+ function finish() { >+ shouldBe('mutations.length', '3'); >+ shouldBe('mutations[0].target', 'div'); >+ shouldBe('mutations[0].type', '"attributes"'); >+ shouldBe('mutations[0].attributeName', '"data-test"'); >+ shouldBe('mutations[0].oldValue', '"foo"'); >+ shouldBe('mutations[1].target', 'div'); >+ shouldBe('mutations[1].type', '"attributes"'); >+ shouldBe('mutations[1].attributeName', '"data-other"'); >+ shouldBe('mutations[1].oldValue', 'null'); >+ shouldBe('mutations[2].target', 'div'); >+ shouldBe('mutations[2].type', '"attributes"'); >+ shouldBe('mutations[2].attributeName', '"id"'); >+ shouldBe('mutations[2].oldValue', '"myId"'); >+ >+ observer.disconnect(); >+ debug(''); >+ runNextTest(); >+ } >+ >+ start(); >+} >+ >+function testMixedNodeAndElementOperations() { >+ var observer; >+ >+ function start() { >+ debug('Test that setAttribute on an attribute with an existing Attr delivers mutation records'); >+ >+ mutations = null; >+ observer = new WebKitMutationObserver(function(mutations) { >+ window.mutations = mutations; >+ }); >+ >+ div = document.createElement('div'); >+ var attr = document.createAttribute('data-test'); >+ attr.value = 'foo'; >+ div.setAttributeNode(attr); >+ observer.observe(div, { attributes: true, attributeOldValue: true }); >+ div.setAttribute('data-test', 'bar'); >+ >+ setTimeout(finish, 0); >+ } >+ >+ function finish() { >+ shouldBe('mutations.length', '1'); >+ shouldBe('mutations[0].target', 'div'); >+ shouldBe('mutations[0].type', '"attributes"'); >+ shouldBe('mutations[0].attributeName', '"data-test"'); >+ shouldBe('mutations[0].oldValue', '"foo"'); >+ >+ observer.disconnect(); >+ debug(''); >+ runNextTest(); >+ } >+ >+ start(); >+} >+ >+function testNamedNodeMapOperations() { >+ var observer; >+ >+ function start() { >+ debug('Test that setNamedItem and removeNamedItem deliver mutation records'); >+ >+ mutations = null; >+ observer = new WebKitMutationObserver(function(mutations) { >+ window.mutations = mutations; >+ }); >+ >+ div = document.createElement('div'); >+ div.setAttribute('data-test', 'foo'); >+ observer.observe(div, { attributes: true, attributeOldValue: true }); >+ var attr = document.createAttribute('data-test'); >+ attr.value = 'bar'; >+ div.attributes.setNamedItem(attr); >+ div.attributes.removeNamedItem('data-test'); >+ >+ setTimeout(finish, 0); >+ } >+ >+ function finish() { >+ shouldBe('mutations.length', '2'); >+ shouldBe('mutations[0].target', 'div'); >+ shouldBe('mutations[0].type', '"attributes"'); >+ shouldBe('mutations[0].attributeName', '"data-test"'); >+ shouldBe('mutations[0].oldValue', '"foo"'); >+ shouldBe('mutations[1].target', 'div'); >+ shouldBe('mutations[1].type', '"attributes"'); >+ shouldBe('mutations[1].attributeName', '"data-test"'); >+ shouldBe('mutations[1].oldValue', '"bar"'); >+ >+ observer.disconnect(); >+ debug(''); >+ runNextTest(); >+ } >+ >+ start(); >+} >+ > var tests = [ > testBasic, > testWrongType, >@@ -804,7 +996,12 @@ var tests = [ > testAttributeFilterNonHTMLDocument, > testStyleAttributePropertyAccess, > testStyleAttributePropertyAccessOldValue, >- testStyleAttributePropertyAccessIgnoreNoop >+ testStyleAttributePropertyAccessIgnoreNoop, >+ testMutateThroughAttrNodeValue, >+ testMutateThroughAttrNodeChild, >+ testSetAndRemoveAttributeNode, >+ testMixedNodeAndElementOperations, >+ testNamedNodeMapOperations > ]; > var testIndex = 0; > >@@ -824,5 +1021,3 @@ else > > </script> > <script src="../js/resources/js-test-post.js"></script> >-</body> >-</html>
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=119085">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 74448
:
119085
|
119275