- Revision
- 103611
- Author
- ad...@chromium.org
- Date
- 2011-12-23 01:12:28 -0800 (Fri, 23 Dec 2011)
Log Message
Minimize callsites and duplication of before/after advice for attribute mutations
https://bugs.webkit.org/show_bug.cgi?id=75054
Reviewed by Ryosuke Niwa.
Source/WebCore:
r103452 helpfully made before and after advice regarding attribute
changes symmetrical. This change finishes that work, by pulling
together all the before/after work, not just the crumbs previously
covered. This includes incrementing Document::domTreeVersion()
when an attribute is about to be changed, Inspector instrumentation,
and MutationEvent dispatch. This is in addition to the previous code,
which handled enqueueing MutationRecords for MutationObservers and
updating the Document's list of IDs.
The only change in behavior should be in InspectorInstrumentation,
which causes DOM breakpoints to occur for more cases of Attribute
mutation. This seems like more correct behavior, and a test has
been included to exercise it.
Hopefully the last Attribute-related refactor for awhile.
* dom/Attr.cpp:
(WebCore::Attr::setValue): Update to call didModifyAttribute instead
of attributeChanged.
* dom/Element.cpp:
(WebCore::Element::removeAttribute): Got rid of
removeAttributeInternal as most of that logic moved back into
NamedNodeMap::removeAttribute.
(WebCore::Element::setAttributeInternal): Reorganized to read better
now that only some cases result in calls to will/didModifyAttribute.
(WebCore::Element::willModifyAttribute): Un-inlined and added
incDOMTreeVersion and InspectorInstrumentation calls.
(WebCore::Element::didModifyAttribute): New method which encapsulates
calling attributeChanged, InspectorInstrumentation, and MutationEvents.
(WebCore::Element::didRemoveAttribute): New method which encapsulates
calling attributeChanged, InspectorInstrumentation, and MutationEvents.
Separate from didModifyAttribute because it has special handling of
the removed Attribute's value.
* dom/Element.h:
(WebCore::Element::willRemoveAttribute): New method which delegates to
willModifyAttribute as appropriate.
* dom/NamedNodeMap.cpp:
(WebCore::NamedNodeMap::setNamedItem): Simplified.
(WebCore::NamedNodeMap::removeNamedItem): Simplified.
(WebCore::NamedNodeMap::addAttribute): Added calls to will/didModifyAttribute.
(WebCore::NamedNodeMap::removeAttribute): ditto.
(WebCore::NamedNodeMap::replaceAttribute): ditto.
* svg/properties/SVGAnimatedPropertySynchronizer.h: Reverted changes
made in r103452 now that addAttribute/removeAttribute once again
call attributeChanged appropriately.
LayoutTests:
* inspector/debugger/dom-breakpoints.html: Add tests for breakpoints
due to mutation of Attr nodes/NamedNodeLists.
* platform/chromium/inspector/debugger/dom-breakpoints-expected.txt:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (103610 => 103611)
--- trunk/LayoutTests/ChangeLog 2011-12-23 08:32:05 UTC (rev 103610)
+++ trunk/LayoutTests/ChangeLog 2011-12-23 09:12:28 UTC (rev 103611)
@@ -1,3 +1,14 @@
+2011-12-23 Adam Klein <ad...@chromium.org>
+
+ Minimize callsites and duplication of before/after advice for attribute mutations
+ https://bugs.webkit.org/show_bug.cgi?id=75054
+
+ Reviewed by Ryosuke Niwa.
+
+ * inspector/debugger/dom-breakpoints.html: Add tests for breakpoints
+ due to mutation of Attr nodes/NamedNodeLists.
+ * platform/chromium/inspector/debugger/dom-breakpoints-expected.txt:
+
2011-12-23 Pierre Rossi <pierre.ro...@gmail.com>
[Qt] REGRESSION(r103467): It broke fast/images/animated-gif-restored-from-bfcache.html
Modified: trunk/LayoutTests/inspector/debugger/dom-breakpoints.html (103610 => 103611)
--- trunk/LayoutTests/inspector/debugger/dom-breakpoints.html 2011-12-23 08:32:05 UTC (rev 103610)
+++ trunk/LayoutTests/inspector/debugger/dom-breakpoints.html 2011-12-23 09:12:28 UTC (rev 103611)
@@ -18,6 +18,20 @@
element.setAttribute(name, value);
}
+function modifyAttrNode(elementId, name, value)
+{
+ var element = document.getElementById(elementId);
+ element.attributes[name].value = value;
+}
+
+function setAttrNode(elementId, name, value)
+{
+ var element = document.getElementById(elementId);
+ var attr = document.createAttribute(name);
+ attr.value = value;
+ element.attributes.setNamedItem(attr);
+}
+
function modifyStyleAttribute(elementId, name, value)
{
var element = document.getElementById(elementId);
@@ -80,10 +94,10 @@
InspectorTest.addResult("Test that 'Attribute Modified' breakpoint is hit when modifying attribute.");
pane._setBreakpoint(rootElement, pane._breakpointTypes.AttributeModified, true);
InspectorTest.addResult("Set 'Attribute Modified' DOM breakpoint on rootElement.");
- InspectorTest.evaluateInPageWithTimeout("modifyAttribute('rootElement', 'className', 'foo')");
- InspectorTest.addResult("Modify rootElement className.");
+ InspectorTest.evaluateInPageWithTimeout("modifyAttribute('rootElement', 'data-test', 'foo')");
+ InspectorTest.addResult("Modify rootElement data-test attribute.");
waitUntilPausedAndDumpStack(step2);
-
+
function step2(callFrames)
{
pane._removeBreakpoint(rootElement, pane._breakpointTypes.AttributeModified);
@@ -91,6 +105,38 @@
}
},
+ function testModifyAttrNode(next)
+ {
+ InspectorTest.addResult("Test that 'Attribute Modified' breakpoint is hit when modifying Attr node.");
+ pane._setBreakpoint(rootElement, pane._breakpointTypes.AttributeModified, true);
+ InspectorTest.addResult("Set 'Attribute Modified' DOM breakpoint on rootElement.");
+ InspectorTest.evaluateInPageWithTimeout("modifyAttrNode('rootElement', 'data-test', 'bar')");
+ InspectorTest.addResult("Modify rootElement data-test attribute.");
+ waitUntilPausedAndDumpStack(step2);
+
+ function step2(callFrames)
+ {
+ pane._removeBreakpoint(rootElement, pane._breakpointTypes.AttributeModified);
+ next();
+ }
+ },
+
+ function testSetAttrNode(next)
+ {
+ InspectorTest.addResult("Test that 'Attribute Modified' breakpoint is hit when adding a new Attr node.");
+ pane._setBreakpoint(rootElement, pane._breakpointTypes.AttributeModified, true);
+ InspectorTest.addResult("Set 'Attribute Modified' DOM breakpoint on rootElement.");
+ InspectorTest.evaluateInPageWithTimeout("setAttrNode('rootElement', 'data-foo', 'bar')");
+ InspectorTest.addResult("Modify rootElement data-foo attribute.");
+ waitUntilPausedAndDumpStack(step2);
+
+ function step2(callFrames)
+ {
+ pane._removeBreakpoint(rootElement, pane._breakpointTypes.AttributeModified);
+ next();
+ }
+ },
+
function testModifyStyleAttribute(next)
{
InspectorTest.addResult("Test that 'Attribute Modified' breakpoint is hit when modifying style attribute.");
Modified: trunk/LayoutTests/platform/chromium/inspector/debugger/dom-breakpoints-expected.txt (103610 => 103611)
--- trunk/LayoutTests/platform/chromium/inspector/debugger/dom-breakpoints-expected.txt 2011-12-23 08:32:05 UTC (rev 103610)
+++ trunk/LayoutTests/platform/chromium/inspector/debugger/dom-breakpoints-expected.txt 2011-12-23 09:12:28 UTC (rev 103611)
@@ -28,7 +28,7 @@
Remove grandchildElement.
Script execution paused.
Call stack:
- 0) removeElement (dom-breakpoints.html:30)
+ 0) removeElement (dom-breakpoints.html:44)
1) (:1)
Paused on a "Subtree Modified" breakpoint set on div#rootElement, because its descendant div#grandchildElement was removed.
Script execution resumed.
@@ -36,7 +36,7 @@
Running: testModifyAttribute
Test that 'Attribute Modified' breakpoint is hit when modifying attribute.
Set 'Attribute Modified' DOM breakpoint on rootElement.
-Modify rootElement className.
+Modify rootElement data-test attribute.
Script execution paused.
Call stack:
0) modifyAttribute (dom-breakpoints.html:18)
@@ -44,13 +44,35 @@
Paused on a "Attribute Modified" breakpoint set on div#rootElement.
Script execution resumed.
+Running: testModifyAttrNode
+Test that 'Attribute Modified' breakpoint is hit when modifying Attr node.
+Set 'Attribute Modified' DOM breakpoint on rootElement.
+Modify rootElement data-test attribute.
+Script execution paused.
+Call stack:
+ 0) modifyAttrNode (dom-breakpoints.html:24)
+ 1) (:1)
+Paused on a "Attribute Modified" breakpoint set on div#rootElement.
+Script execution resumed.
+
+Running: testSetAttrNode
+Test that 'Attribute Modified' breakpoint is hit when adding a new Attr node.
+Set 'Attribute Modified' DOM breakpoint on rootElement.
+Modify rootElement data-foo attribute.
+Script execution paused.
+Call stack:
+ 0) setAttrNode (dom-breakpoints.html:32)
+ 1) (:1)
+Paused on a "Attribute Modified" breakpoint set on div#rootElement.
+Script execution resumed.
+
Running: testModifyStyleAttribute
Test that 'Attribute Modified' breakpoint is hit when modifying style attribute.
Set 'Attribute Modified' DOM breakpoint on rootElement.
Modify rootElement style.color attribute.
Script execution paused.
Call stack:
- 0) modifyStyleAttribute (dom-breakpoints.html:24)
+ 0) modifyStyleAttribute (dom-breakpoints.html:38)
1) (:1)
Paused on a "Attribute Modified" breakpoint set on div#rootElement.
Script execution resumed.
@@ -61,7 +83,7 @@
Remove elementToRemove.
Script execution paused.
Call stack:
- 0) removeElement (dom-breakpoints.html:30)
+ 0) removeElement (dom-breakpoints.html:44)
1) (:1)
Paused on a "Node Removed" breakpoint set on div#elementToRemove.
Script execution resumed.
Modified: trunk/Source/WebCore/ChangeLog (103610 => 103611)
--- trunk/Source/WebCore/ChangeLog 2011-12-23 08:32:05 UTC (rev 103610)
+++ trunk/Source/WebCore/ChangeLog 2011-12-23 09:12:28 UTC (rev 103611)
@@ -1,3 +1,56 @@
+2011-12-23 Adam Klein <ad...@chromium.org>
+
+ Minimize callsites and duplication of before/after advice for attribute mutations
+ https://bugs.webkit.org/show_bug.cgi?id=75054
+
+ Reviewed by Ryosuke Niwa.
+
+ r103452 helpfully made before and after advice regarding attribute
+ changes symmetrical. This change finishes that work, by pulling
+ together all the before/after work, not just the crumbs previously
+ covered. This includes incrementing Document::domTreeVersion()
+ when an attribute is about to be changed, Inspector instrumentation,
+ and MutationEvent dispatch. This is in addition to the previous code,
+ which handled enqueueing MutationRecords for MutationObservers and
+ updating the Document's list of IDs.
+
+ The only change in behavior should be in InspectorInstrumentation,
+ which causes DOM breakpoints to occur for more cases of Attribute
+ mutation. This seems like more correct behavior, and a test has
+ been included to exercise it.
+
+ Hopefully the last Attribute-related refactor for awhile.
+
+ * dom/Attr.cpp:
+ (WebCore::Attr::setValue): Update to call didModifyAttribute instead
+ of attributeChanged.
+ * dom/Element.cpp:
+ (WebCore::Element::removeAttribute): Got rid of
+ removeAttributeInternal as most of that logic moved back into
+ NamedNodeMap::removeAttribute.
+ (WebCore::Element::setAttributeInternal): Reorganized to read better
+ now that only some cases result in calls to will/didModifyAttribute.
+ (WebCore::Element::willModifyAttribute): Un-inlined and added
+ incDOMTreeVersion and InspectorInstrumentation calls.
+ (WebCore::Element::didModifyAttribute): New method which encapsulates
+ calling attributeChanged, InspectorInstrumentation, and MutationEvents.
+ (WebCore::Element::didRemoveAttribute): New method which encapsulates
+ calling attributeChanged, InspectorInstrumentation, and MutationEvents.
+ Separate from didModifyAttribute because it has special handling of
+ the removed Attribute's value.
+ * dom/Element.h:
+ (WebCore::Element::willRemoveAttribute): New method which delegates to
+ willModifyAttribute as appropriate.
+ * dom/NamedNodeMap.cpp:
+ (WebCore::NamedNodeMap::setNamedItem): Simplified.
+ (WebCore::NamedNodeMap::removeNamedItem): Simplified.
+ (WebCore::NamedNodeMap::addAttribute): Added calls to will/didModifyAttribute.
+ (WebCore::NamedNodeMap::removeAttribute): ditto.
+ (WebCore::NamedNodeMap::replaceAttribute): ditto.
+ * svg/properties/SVGAnimatedPropertySynchronizer.h: Reverted changes
+ made in r103452 now that addAttribute/removeAttribute once again
+ call attributeChanged appropriately.
+
2011-12-22 Matt Falkenhagen <fal...@chromium.org>
Map 'lang' and xml:lang attributes to '-webkit-locale' CSS property for use with font fallback and text-transform
Modified: trunk/Source/WebCore/dom/Attr.cpp (103610 => 103611)
--- trunk/Source/WebCore/dom/Attr.cpp 2011-12-23 08:32:05 UTC (rev 103610)
+++ trunk/Source/WebCore/dom/Attr.cpp 2011-12-23 09:12:28 UTC (rev 103611)
@@ -140,7 +140,7 @@
setValue(value);
if (m_element)
- m_element->attributeChanged(m_attribute.get());
+ m_element->didModifyAttribute(m_attribute.get());
}
void Attr::setNodeValue(const String& v, ExceptionCode& ec)
Modified: trunk/Source/WebCore/dom/Element.cpp (103610 => 103611)
--- trunk/Source/WebCore/dom/Element.cpp 2011-12-23 08:32:05 UTC (rev 103610)
+++ trunk/Source/WebCore/dom/Element.cpp 2011-12-23 09:12:28 UTC (rev 103611)
@@ -178,52 +178,14 @@
{
}
-#if ENABLE(MUTATION_OBSERVERS)
-void Element::enqueueAttributesMutationRecordIfRequested(const QualifiedName& attributeName, const AtomicString& oldValue)
-{
- if (isSynchronizingStyleAttribute())
- return;
- if (OwnPtr<MutationObserverInterestGroup> mutationRecipients = MutationObserverInterestGroup::createForAttributesMutation(this, attributeName))
- mutationRecipients->enqueueMutationRecord(MutationRecord::createAttributes(this, attributeName, oldValue));
-}
-#endif
-
void Element::removeAttribute(const QualifiedName& name)
{
if (!m_attributeMap)
return;
- removeAttributeInternal(m_attributeMap->getAttributeItemIndex(name));
+ m_attributeMap->removeAttribute(name);
}
-inline void Element::removeAttributeInternal(size_t index)
-{
- ASSERT(m_attributeMap);
- if (index == notFound)
- return;
-
- InspectorInstrumentation::willModifyDOMAttr(document(), this);
-
- RefPtr<Attribute> attribute = m_attributeMap->attributeItem(index);
-
- if (!attribute->isNull())
- willModifyAttribute(attribute->name(), attribute->value(), nullAtom);
-
- m_attributeMap->removeAttribute(index);
-
- if (!attribute->isNull()) {
- AtomicString savedValue = attribute->value();
- attribute->setValue(nullAtom);
- attributeChanged(attribute.get());
- attribute->setValue(savedValue);
- }
-
- InspectorInstrumentation::didRemoveDOMAttr(document(), this, attribute->name().localName());
-
- if (!isSynchronizingStyleAttribute())
- dispatchSubtreeModifiedEvent();
-}
-
void Element::setBooleanAttribute(const QualifiedName& name, bool value)
{
if (value)
@@ -663,36 +625,26 @@
inline void Element::setAttributeInternal(size_t index, const QualifiedName& name, const AtomicString& value)
{
+ Attribute* old = index != notFound ? m_attributeMap->attributeItem(index) : 0;
if (value.isNull()) {
- removeAttributeInternal(index);
+ if (old)
+ m_attributeMap->removeAttribute(index);
return;
}
- Attribute* old = index != notFound ? m_attributeMap->attributeItem(index) : 0;
+ if (!old) {
+ m_attributeMap->addAttribute(createAttribute(name, value));
+ return;
+ }
- if (!isSynchronizingStyleAttribute())
- InspectorInstrumentation::willModifyDOMAttr(document(), this);
-
- document()->incDOMTreeVersion();
-
willModifyAttribute(name, old ? old->value() : nullAtom, value);
- if (!old) {
- RefPtr<Attribute> attribute = createAttribute(name, value);
- m_attributeMap->addAttribute(attribute);
- attributeChanged(attribute.get());
- } else {
- if (Attr* attrNode = old->attr())
- attrNode->setValue(value);
- else
- old->setValue(value);
- attributeChanged(old);
- }
+ if (Attr* attrNode = old->attr())
+ attrNode->setValue(value);
+ else
+ old->setValue(value);
- if (!isSynchronizingStyleAttribute()) {
- InspectorInstrumentation::didModifyDOMAttr(document(), this, name.localName(), value);
- dispatchSubtreeModifiedEvent();
- }
+ didModifyAttribute(old);
}
PassRefPtr<Attribute> Element::createAttribute(const QualifiedName& name, const AtomicString& value)
@@ -1494,7 +1446,11 @@
return;
String localName = shouldIgnoreAttributeCase(this) ? name.lower() : name;
- removeAttributeInternal(m_attributeMap->getAttributeItemIndex(localName, false));
+ size_t index = m_attributeMap->getAttributeItemIndex(localName, false);
+ if (index == notFound)
+ return;
+
+ m_attributeMap->removeAttribute(index);
}
void Element::removeAttributeNS(const String& namespaceURI, const String& localName)
@@ -1992,4 +1948,51 @@
}
#endif
+void Element::willModifyAttribute(const QualifiedName& name, const AtomicString& oldValue, const AtomicString& newValue)
+{
+ document()->incDOMTreeVersion();
+
+ if (isIdAttributeName(name))
+ updateId(oldValue, newValue);
+
+#if ENABLE(MUTATION_OBSERVERS)
+ if (!isSynchronizingStyleAttribute()) {
+ if (OwnPtr<MutationObserverInterestGroup> recipients = MutationObserverInterestGroup::createForAttributesMutation(this, name))
+ recipients->enqueueMutationRecord(MutationRecord::createAttributes(this, name, oldValue));
+ }
+#endif
+
+#if ENABLE(INSPECTOR)
+ if (!isSynchronizingStyleAttribute())
+ InspectorInstrumentation::willModifyDOMAttr(document(), this);
+#endif
+}
+
+void Element::didModifyAttribute(Attribute* attr)
+{
+ attributeChanged(attr);
+
+ if (!isSynchronizingStyleAttribute()) {
+ InspectorInstrumentation::didModifyDOMAttr(document(), this, attr->name().localName(), attr->value());
+ dispatchSubtreeModifiedEvent();
+ }
+}
+
+void Element::didRemoveAttribute(Attribute* attr)
+{
+ if (attr->isNull())
+ return;
+
+ AtomicString savedValue = attr->value();
+ attr->setValue(nullAtom);
+ attributeChanged(attr);
+ attr->setValue(savedValue);
+
+ if (!isSynchronizingStyleAttribute()) {
+ InspectorInstrumentation::didRemoveDOMAttr(document(), this, attr->name().localName());
+ dispatchSubtreeModifiedEvent();
+ }
+}
+
+
} // namespace WebCore
Modified: trunk/Source/WebCore/dom/Element.h (103610 => 103611)
--- trunk/Source/WebCore/dom/Element.h 2011-12-23 08:32:05 UTC (rev 103610)
+++ trunk/Source/WebCore/dom/Element.h 2011-12-23 09:12:28 UTC (rev 103611)
@@ -265,7 +265,11 @@
virtual String title() const;
void updateId(const AtomicString& oldId, const AtomicString& newId);
+
void willModifyAttribute(const QualifiedName&, const AtomicString& oldValue, const AtomicString& newValue);
+ void willRemoveAttribute(const QualifiedName&, const AtomicString& value);
+ void didModifyAttribute(Attribute*);
+ void didRemoveAttribute(Attribute*);
LayoutSize minimumSizeForResizing() const;
void setMinimumSizeForResizing(const LayoutSize&);
@@ -392,7 +396,6 @@
virtual bool childTypeAllowed(NodeType) const;
void setAttributeInternal(size_t index, const QualifiedName&, const AtomicString& value);
- void removeAttributeInternal(size_t index);
virtual PassRefPtr<Attribute> createAttribute(const QualifiedName&, const AtomicString& value);
#ifndef NDEBUG
@@ -429,10 +432,6 @@
SpellcheckAttributeState spellcheckAttributeState() const;
-#if ENABLE(MUTATION_OBSERVERS)
- void enqueueAttributesMutationRecordIfRequested(const QualifiedName&, const AtomicString& oldValue);
-#endif
-
private:
mutable RefPtr<NamedNodeMap> m_attributeMap;
};
@@ -514,16 +513,10 @@
scope->addElementById(newId, this);
}
-inline void Element::willModifyAttribute(const QualifiedName& name, const AtomicString& oldValue, const AtomicString& newValue)
+inline void Element::willRemoveAttribute(const QualifiedName& name, const AtomicString& value)
{
- if (isIdAttributeName(name))
- updateId(oldValue, newValue);
-
- // FIXME: Should probably call InspectorInstrumentation::willModifyDOMAttr here.
-
-#if ENABLE(MUTATION_OBSERVERS)
- enqueueAttributesMutationRecordIfRequested(name, oldValue);
-#endif
+ if (!value.isNull())
+ willModifyAttribute(name, value, nullAtom);
}
inline bool Element::fastHasAttribute(const QualifiedName& name) const
Modified: trunk/Source/WebCore/dom/NamedNodeMap.cpp (103610 => 103611)
--- trunk/Source/WebCore/dom/NamedNodeMap.cpp 2011-12-23 08:32:05 UTC (rev 103610)
+++ trunk/Source/WebCore/dom/NamedNodeMap.cpp 2011-12-23 09:12:28 UTC (rev 103611)
@@ -120,8 +120,6 @@
return 0;
}
- m_element->willModifyAttribute(attribute->name(), oldAttribute ? oldAttribute->value() : nullAtom, attribute->value());
-
RefPtr<Attr> oldAttr;
if (oldAttribute) {
oldAttr = oldAttribute->createAttrIfNeeded(m_element);
@@ -129,11 +127,6 @@
} else
addAttribute(attribute);
- m_element->attributeChanged(attribute);
-
- if (attribute->name() != styleAttr)
- m_element->dispatchSubtreeModifiedEvent();
-
return oldAttr.release();
}
@@ -152,24 +145,10 @@
return 0;
}
- RefPtr<Attribute> attribute = attributeItem(index);
- RefPtr<Attr> attr = attribute->createAttrIfNeeded(m_element);
+ RefPtr<Attr> attr = m_attributes[index]->createAttrIfNeeded(m_element);
- if (!attribute->isNull())
- m_element->willModifyAttribute(attribute->name(), attribute->value(), nullAtom);
-
removeAttribute(index);
- if (!attribute->isNull()) {
- AtomicString savedValue = attribute->value();
- attribute->setValue(nullAtom);
- m_element->attributeChanged(attribute.get());
- attribute->setValue(savedValue);
- }
-
- if (attribute->name() != styleAttr)
- m_element->dispatchSubtreeModifiedEvent();
-
return attr.release();
}
@@ -254,32 +233,56 @@
m_element->attributeChanged(m_attributes[i].get(), true);
}
-void NamedNodeMap::addAttribute(PassRefPtr<Attribute> attribute)
+void NamedNodeMap::addAttribute(PassRefPtr<Attribute> prpAttribute)
{
+ RefPtr<Attribute> attribute = prpAttribute;
+
+ if (m_element)
+ m_element->willModifyAttribute(attribute->name(), nullAtom, attribute->value());
+
m_attributes.append(attribute);
- if (Attr* attr = m_attributes.last()->attr())
+ if (Attr* attr = attribute->attr())
attr->m_element = m_element;
+
+ if (m_element)
+ m_element->didModifyAttribute(attribute.get());
}
void NamedNodeMap::removeAttribute(size_t index)
{
ASSERT(index < length());
- if (Attr* attr = m_attributes[index]->attr())
+ RefPtr<Attribute> attribute = m_attributes[index];
+
+ if (m_element)
+ m_element->willRemoveAttribute(attribute->name(), attribute->value());
+
+ if (Attr* attr = attribute->attr())
attr->m_element = 0;
+ m_attributes.remove(index);
- m_attributes.remove(index);
+ if (m_element)
+ m_element->didRemoveAttribute(attribute.get());
}
-void NamedNodeMap::replaceAttribute(size_t index, PassRefPtr<Attribute> attribute)
+void NamedNodeMap::replaceAttribute(size_t index, PassRefPtr<Attribute> prpAttribute)
{
ASSERT(index < length());
- if (Attr* attr = m_attributes[index]->attr())
+ RefPtr<Attribute> attribute = prpAttribute;
+ Attribute* old = m_attributes[index].get();
+
+ if (m_element)
+ m_element->willModifyAttribute(attribute->name(), old->value(), attribute->value());
+
+ if (Attr* attr = old->attr())
attr->m_element = 0;
m_attributes[index] = attribute;
- if (Attr* attr = m_attributes[index]->attr())
+ if (Attr* attr = attribute->attr())
attr->m_element = m_element;
+
+ if (m_element)
+ m_element->didModifyAttribute(attribute.get());
}
void NamedNodeMap::setClass(const String& classStr)
Modified: trunk/Source/WebCore/svg/properties/SVGAnimatedPropertySynchronizer.h (103610 => 103611)
--- trunk/Source/WebCore/svg/properties/SVGAnimatedPropertySynchronizer.h 2011-12-23 08:32:05 UTC (rev 103610)
+++ trunk/Source/WebCore/svg/properties/SVGAnimatedPropertySynchronizer.h 2011-12-23 09:12:28 UTC (rev 103611)
@@ -34,16 +34,18 @@
static void synchronize(SVGElement* ownerElement, const QualifiedName& attrName, const AtomicString& value)
{
// If the attribute already exists on the element, we change the
- // Attribute directly rather than going through Element::setAttribute to
- // avoid a call to Element::attributeChanged that could cause the
- // SVGElement to erroneously reset its properties.
+ // Attribute directly to avoid a call to Element::attributeChanged
+ // that could cause the SVGElement to erroneously reset its properties.
// svg/dom/SVGStringList-basics.xhtml exercises this behavior.
NamedNodeMap* namedAttrMap = ownerElement->attributes(false);
Attribute* old = namedAttrMap->getAttributeItem(attrName);
- if (old)
+ if (old && value.isNull())
+ namedAttrMap->removeAttribute(old->name());
+ else if (!old && !value.isNull())
+ namedAttrMap->addAttribute(ownerElement->createAttribute(attrName, value));
+ else if (old && !value.isNull())
old->setValue(value);
- else
- ownerElement->setAttribute(attrName, value);
+
}
};
