Diff
Modified: trunk/LayoutTests/ChangeLog (101100 => 101101)
--- trunk/LayoutTests/ChangeLog 2011-11-23 20:18:15 UTC (rev 101100)
+++ trunk/LayoutTests/ChangeLog 2011-11-23 21:40:06 UTC (rev 101101)
@@ -1,3 +1,15 @@
+2011-11-23 Rafael Weinstein <[email protected]>
+
+ [MutationObservers] Modifications to the style property don't dispatch "attributes" Mutation Records
+ https://bugs.webkit.org/show_bug.cgi?id=70137
+
+ Reviewed by Ryosuke Niwa.
+
+ Added tests asserting that changes to the style property are observable and work correctly if oldValue is requested.
+
+ * fast/mutation/observe-attributes-expected.txt:
+ * fast/mutation/observe-attributes.html:
+
2011-11-23 Adam Klein <[email protected]>
Rebaseline Chromium after r101056. Unreviewed gardening.
Modified: trunk/LayoutTests/fast/mutation/observe-attributes-expected.txt (101100 => 101101)
--- trunk/LayoutTests/fast/mutation/observe-attributes-expected.txt 2011-11-23 20:18:15 UTC (rev 101100)
+++ trunk/LayoutTests/fast/mutation/observe-attributes-expected.txt 2011-11-23 21:40:06 UTC (rev 101101)
@@ -152,6 +152,34 @@
PASS mutations[3].attributeName is "pathLength"
PASS mutations[3].attributeNamespace is "http://www.w3.org/2000/svg"
+Testing that modifying an elements style property dispatches Mutation Records.
+PASS mutations.length is 3
+PASS mutations[0].type is "attributes"
+PASS mutations[0].attributeName is "style"
+PASS mutations[0].oldValue is null
+PASS mutations[1].type is "attributes"
+PASS mutations[1].attributeName is "style"
+PASS mutations[1].oldValue is null
+PASS mutations[2].type is "attributes"
+PASS mutations[2].attributeName is "style"
+PASS mutations[2].oldValue is null
+...mutation record created.
+PASS mutations is null
+
+Testing that modifying an elements style property dispatches Mutation Records with correct oldValues.
+PASS mutations.length is 3
+PASS mutations[0].type is "attributes"
+PASS mutations[0].attributeName is "style"
+PASS mutations[0].oldValue is "color: yellow; width: 100px; "
+PASS mutations[1].type is "attributes"
+PASS mutations[1].attributeName is "style"
+PASS mutations[1].oldValue is "width: 100px; color: red; "
+PASS mutations[2].type is "attributes"
+PASS mutations[2].attributeName is "style"
+PASS mutations[2].oldValue is "color: red; width: 200px; "
+...mutation record created.
+PASS mutations is null
+
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/fast/mutation/observe-attributes.html (101100 => 101101)
--- trunk/LayoutTests/fast/mutation/observe-attributes.html 2011-11-23 20:18:15 UTC (rev 101100)
+++ trunk/LayoutTests/fast/mutation/observe-attributes.html 2011-11-23 21:40:06 UTC (rev 101101)
@@ -638,6 +638,110 @@
start();
}
+function testStyleAttributePropertyAccess() {
+ var svgDoc, div, path;
+ var observer;
+
+ function start() {
+ debug('Testing that modifying an elements style property dispatches Mutation Records.');
+
+ mutations = null;
+ observer = new WebKitMutationObserver(function(m) {
+ mutations = m;
+ });
+
+ div = document.createElement('div');
+ div.setAttribute('style', 'color: yellow; width: 100px; ');
+ observer.observe(div, { attributes: true });
+ div.style.color = 'red';
+ div.style.width = '200px';
+ div.style.color = 'blue';
+
+ setTimeout(checkAndContinue, 0);
+ }
+
+ function checkAndContinue() {
+ shouldBe('mutations.length', '3');
+ shouldBe('mutations[0].type', '"attributes"');
+ shouldBe('mutations[0].attributeName', '"style"');
+ shouldBe('mutations[0].oldValue', 'null');
+ shouldBe('mutations[1].type', '"attributes"');
+ shouldBe('mutations[1].attributeName', '"style"');
+ shouldBe('mutations[1].oldValue', 'null');
+ shouldBe('mutations[2].type', '"attributes"');
+ shouldBe('mutations[2].attributeName', '"style"');
+ shouldBe('mutations[2].oldValue', 'null');
+
+ mutations = null;
+ div.getAttribute('style');
+ setTimeout(finish, 0);
+ }
+
+ function finish() {
+ debug('...mutation record created.');
+
+ shouldBe('mutations', 'null');
+
+ observer.disconnect();
+ debug('');
+ runNextTest();
+ }
+
+ start();
+}
+
+function testStyleAttributePropertyAccessOldValue() {
+ var svgDoc, div, path;
+ var observer;
+
+ function start() {
+ debug('Testing that modifying an elements style property dispatches Mutation Records with correct oldValues.');
+
+ mutations = null;
+ observer = new WebKitMutationObserver(function(m) {
+ mutations = m;
+ });
+
+ div = document.createElement('div');
+ div.setAttribute('style', 'color: yellow; width: 100px; ');
+ observer.observe(div, { attributes: true, attributeOldValue: true });
+ div.style.color = 'red';
+ div.style.width = '200px';
+ div.style.color = 'blue';
+
+ setTimeout(checkAndContinue, 0);
+ }
+
+ function checkAndContinue() {
+ shouldBe('mutations.length', '3');
+ shouldBe('mutations[0].type', '"attributes"');
+ shouldBe('mutations[0].attributeName', '"style"');
+ shouldBe('mutations[0].oldValue', '"color: yellow; width: 100px; "');
+ shouldBe('mutations[1].type', '"attributes"');
+ shouldBe('mutations[1].attributeName', '"style"');
+ shouldBe('mutations[1].oldValue', '"width: 100px; color: red; "');
+ shouldBe('mutations[2].type', '"attributes"');
+ shouldBe('mutations[2].attributeName', '"style"');
+ shouldBe('mutations[2].oldValue', '"color: red; width: 200px; "');
+
+ mutations = null;
+ div.getAttribute('style');
+ setTimeout(finish, 0);
+ }
+
+ function finish() {
+ debug('...mutation record created.');
+
+ shouldBe('mutations', 'null');
+
+ observer.disconnect();
+ debug('');
+ runNextTest();
+ }
+
+ start();
+}
+
var tests = [
testBasic,
testWrongType,
@@ -653,7 +757,9 @@
testAttributeFilter,
testAttributeFilterSubtree,
testAttributeFilterNonHTMLElement,
- testAttributeFilterNonHTMLDocument
+ testAttributeFilterNonHTMLDocument,
+ testStyleAttributePropertyAccess,
+ testStyleAttributePropertyAccessOldValue
];
var testIndex = 0;
Modified: trunk/Source/WebCore/ChangeLog (101100 => 101101)
--- trunk/Source/WebCore/ChangeLog 2011-11-23 20:18:15 UTC (rev 101100)
+++ trunk/Source/WebCore/ChangeLog 2011-11-23 21:40:06 UTC (rev 101101)
@@ -1,3 +1,28 @@
+2011-11-23 Rafael Weinstein <[email protected]>
+
+ [MutationObservers] Modifications to the style property don't dispatch "attributes" Mutation Records
+ https://bugs.webkit.org/show_bug.cgi?id=70137
+
+ Reviewed by Ryosuke Niwa.
+
+ This patch adds a private AttributesMutationScope mechanism to CSSMutableStyleDeclaration (which uses
+ the RAII pattern similar to the public ChildListMutationScope). This manages the (sometimes conditional)
+ pre-change serialization of the style attribute (if an observer has requested oldValue), creation of
+ the mutation record, and dispatch if the declaration was actual affected.
+
+ * css/CSSMutableStyleDeclaration.cpp:
+ (WebCore::CSSMutableStyleDeclaration::removeProperty):
+ (WebCore::CSSMutableStyleDeclaration::setProperty):
+ (WebCore::CSSMutableStyleDeclaration::setPropertyInternal):
+ (WebCore::CSSMutableStyleDeclaration::parseDeclaration):
+ (WebCore::CSSMutableStyleDeclaration::addParsedProperties):
+ (WebCore::CSSMutableStyleDeclaration::addParsedProperty):
+ (WebCore::CSSMutableStyleDeclaration::setCssText):
+ (WebCore::CSSMutableStyleDeclaration::merge):
+ (WebCore::CSSMutableStyleDeclaration::removePropertiesInSet):
+ * dom/Element.cpp:
+ (WebCore::Element::setAttribute):
+
2011-11-23 Dmitry Lomov <[email protected]>
Unreviewed, rebaseline binding tests after http://trac.webkit.org/changeset/101064.
Modified: trunk/Source/WebCore/css/CSSMutableStyleDeclaration.cpp (101100 => 101101)
--- trunk/Source/WebCore/css/CSSMutableStyleDeclaration.cpp 2011-11-23 20:18:15 UTC (rev 101100)
+++ trunk/Source/WebCore/css/CSSMutableStyleDeclaration.cpp 2011-11-23 21:40:06 UTC (rev 101101)
@@ -32,8 +32,11 @@
#include "CSSValueList.h"
#include "Document.h"
#include "ExceptionCode.h"
+#include "HTMLNames.h"
#include "InspectorInstrumentation.h"
+#include "MutationRecord.h"
#include "StyledElement.h"
+#include "WebKitMutationObserver.h"
#include <wtf/text/StringBuilder.h>
#include <wtf/text/WTFString.h>
@@ -41,6 +44,69 @@
namespace WebCore {
+#if ENABLE(MUTATION_OBSERVERS)
+namespace {
+
+class StyleAttributeMutationScope {
+ WTF_MAKE_NONCOPYABLE(StyleAttributeMutationScope);
+public:
+ StyleAttributeMutationScope(CSSMutableStyleDeclaration* decl)
+ {
+ ++s_scopeCount;
+
+ if (s_scopeCount != 1) {
+ ASSERT(s_currentDecl == decl);
+ return;
+ }
+
+ ASSERT(!s_currentDecl);
+ s_currentDecl = decl;
+
+ if (!s_currentDecl->isInlineStyleDeclaration())
+ return;
+
+ s_mutationRecipients = MutationObserverInterestGroup::createForAttributesMutation(s_currentDecl->node(), HTMLNames::styleAttr);
+ if (s_mutationRecipients->isEmpty()) {
+ s_mutationRecipients.clear();
+ return;
+ }
+
+ Element* element = toElement(s_currentDecl->node());
+ AtomicString oldValue = s_mutationRecipients->isOldValueRequested() ? element->getAttribute(HTMLNames::styleAttr) : nullAtom;
+ s_mutation = MutationRecord::createAttributes(element, HTMLNames::styleAttr, oldValue);
+ }
+
+ ~StyleAttributeMutationScope()
+ {
+ --s_scopeCount;
+ if (!s_scopeCount)
+ s_currentDecl = 0;
+ }
+
+ void enqueueMutationRecord()
+ {
+ if (!s_mutation)
+ return;
+ s_mutationRecipients->enqueueMutationRecord(s_mutation);
+ s_mutation.clear();
+ s_mutationRecipients.clear();
+ }
+
+private:
+ static unsigned s_scopeCount;
+ static OwnPtr<MutationObserverInterestGroup> s_mutationRecipients;
+ static RefPtr<MutationRecord> s_mutation;
+ static CSSMutableStyleDeclaration* s_currentDecl;
+};
+
+unsigned StyleAttributeMutationScope::s_scopeCount = 0;
+OwnPtr<MutationObserverInterestGroup> StyleAttributeMutationScope::s_mutationRecipients;
+RefPtr<MutationRecord> StyleAttributeMutationScope::s_mutation;
+CSSMutableStyleDeclaration* StyleAttributeMutationScope::s_currentDecl = 0;
+
+} // namespace
+#endif // ENABLE(MUTATION_OBSERVERS)
+
CSSMutableStyleDeclaration::CSSMutableStyleDeclaration()
: CSSStyleDeclaration(0, /* isMutable */ true)
, m_node(0)
@@ -520,6 +586,10 @@
{
ASSERT(!m_iteratorCount);
+#if ENABLE(MUTATION_OBSERVERS)
+ StyleAttributeMutationScope mutationScope(this);
+#endif
+
if (removeShorthandProperty(propertyID, notifyChanged)) {
// FIXME: Return an equivalent shorthand when possible.
return String();
@@ -535,6 +605,10 @@
// and sweeping them when the vector grows too big.
m_properties.remove(foundProperty - m_properties.data());
+#if ENABLE(MUTATION_OBSERVERS)
+ mutationScope.enqueueMutationRecord();
+#endif
+
if (notifyChanged)
setNeedsStyleRecalc();
@@ -602,6 +676,10 @@
{
ASSERT(!m_iteratorCount);
+#if ENABLE(MUTATION_OBSERVERS)
+ StyleAttributeMutationScope mutationScope(this);
+#endif
+
// Setting the value to an empty string just removes the property in both IE and Gecko.
// Setting it to null seems to produce less consistent results, but we treat it just the same.
if (value.isEmpty()) {
@@ -615,16 +693,27 @@
if (!success) {
// CSS DOM requires raising SYNTAX_ERR here, but this is too dangerous for compatibility,
// see <http://bugs.webkit.org/show_bug.cgi?id=7296>.
- } else if (notifyChanged)
+ return false;
+ }
+
+#if ENABLE(MUTATION_OBSERVERS)
+ mutationScope.enqueueMutationRecord();
+#endif
+
+ if (notifyChanged)
setNeedsStyleRecalc();
- return success;
+ return true;
}
void CSSMutableStyleDeclaration::setPropertyInternal(const CSSProperty& property, CSSProperty* slot)
{
ASSERT(!m_iteratorCount);
+#if ENABLE(MUTATION_OBSERVERS)
+ StyleAttributeMutationScope mutationScope(this);
+#endif
+
if (!removeShorthandProperty(property.id(), false)) {
CSSProperty* toReplace = slot ? slot : findPropertyWithId(property.id());
if (toReplace) {
@@ -633,6 +722,10 @@
}
}
m_properties.append(property);
+
+#if ENABLE(MUTATION_OBSERVERS)
+ mutationScope.enqueueMutationRecord();
+#endif
}
bool CSSMutableStyleDeclaration::setProperty(int propertyID, int value, bool important, bool notifyChanged)
@@ -673,9 +766,18 @@
{
ASSERT(!m_iteratorCount);
+#if ENABLE(MUTATION_OBSERVERS)
+ StyleAttributeMutationScope mutationScope(this);
+#endif
+
m_properties.clear();
CSSParser parser(useStrictParsing());
parser.parseDeclaration(this, styleDeclaration);
+
+#if ENABLE(MUTATION_OBSERVERS)
+ mutationScope.enqueueMutationRecord();
+#endif
+
setNeedsStyleRecalc();
}
@@ -683,10 +785,18 @@
{
ASSERT(!m_iteratorCount);
+#if ENABLE(MUTATION_OBSERVERS)
+ StyleAttributeMutationScope mutationScope(this);
+#endif
+
m_properties.reserveCapacity(numProperties);
for (int i = 0; i < numProperties; ++i)
addParsedProperty(*properties[i]);
+#if ENABLE(MUTATION_OBSERVERS)
+ mutationScope.enqueueMutationRecord();
+#endif
+
// FIXME: This probably should have a call to setNeedsStyleRecalc() if something changed. We may also wish to add
// a notifyChanged argument to this function to follow the model of other functions in this class.
}
@@ -695,11 +805,19 @@
{
ASSERT(!m_iteratorCount);
+#if ENABLE(MUTATION_OBSERVERS)
+ StyleAttributeMutationScope mutationScope(this);
+#endif
+
// Only add properties that have no !important counterpart present
if (!getPropertyPriority(property.id()) || property.isImportant()) {
removeProperty(property.id(), false, false);
m_properties.append(property);
}
+
+#if ENABLE(MUTATION_OBSERVERS)
+ mutationScope.enqueueMutationRecord();
+#endif
}
void CSSMutableStyleDeclaration::setLengthProperty(int propertyId, const String& value, bool important, bool /*multiLength*/)
@@ -790,10 +908,19 @@
{
ASSERT(!m_iteratorCount);
+#if ENABLE(MUTATION_OBSERVERS)
+ StyleAttributeMutationScope mutationScope(this);
+#endif
+
ec = 0;
m_properties.clear();
CSSParser parser(useStrictParsing());
parser.parseDeclaration(this, text);
+
+#if ENABLE(MUTATION_OBSERVERS)
+ mutationScope.enqueueMutationRecord();
+#endif
+
// FIXME: Detect syntax errors and set ec.
setNeedsStyleRecalc();
}
@@ -802,6 +929,10 @@
{
ASSERT(!m_iteratorCount);
+#if ENABLE(MUTATION_OBSERVERS)
+ StyleAttributeMutationScope mutationScope(this);
+#endif
+
unsigned size = other->m_properties.size();
for (unsigned n = 0; n < size; ++n) {
const CSSProperty& toMerge = other->m_properties[n];
@@ -813,6 +944,11 @@
} else
m_properties.append(toMerge);
}
+
+#if ENABLE(MUTATION_OBSERVERS)
+ mutationScope.enqueueMutationRecord();
+#endif
+
// FIXME: This probably should have a call to setNeedsStyleRecalc() if something changed. We may also wish to add
// a notifyChanged argument to this function to follow the model of other functions in this class.
}
@@ -870,6 +1006,10 @@
if (m_properties.isEmpty())
return;
+#if ENABLE(MUTATION_OBSERVERS)
+ StyleAttributeMutationScope mutationScope(this);
+#endif
+
// FIXME: This is always used with static sets and in that case constructing the hash repeatedly is pretty pointless.
HashSet<int> toRemove;
for (unsigned i = 0; i < length; ++i)
@@ -892,8 +1032,14 @@
bool changed = newProperties.size() != m_properties.size();
m_properties = newProperties;
- if (changed && notifyChanged)
- setNeedsStyleRecalc();
+ if (!changed || !notifyChanged)
+ return;
+
+#if ENABLE(MUTATION_OBSERVERS)
+ mutationScope.enqueueMutationRecord();
+#endif
+
+ setNeedsStyleRecalc();
}
PassRefPtr<CSSMutableStyleDeclaration> CSSMutableStyleDeclaration::makeMutable()
Modified: trunk/Source/WebCore/dom/Element.cpp (101100 => 101101)
--- trunk/Source/WebCore/dom/Element.cpp 2011-11-23 20:18:15 UTC (rev 101100)
+++ trunk/Source/WebCore/dom/Element.cpp 2011-11-23 21:40:06 UTC (rev 101101)
@@ -619,7 +619,7 @@
#if ENABLE(MUTATION_OBSERVERS)
static void enqueueAttributesMutationRecord(Element* target, const QualifiedName& attributeName, const AtomicString& oldValue)
{
- OwnPtr<MutationObserverInterestGroup> mutationRecipients = MutationObserverInterestGroup::createForAttributesMutation(target, attributeName.localName());
+ OwnPtr<MutationObserverInterestGroup> mutationRecipients = MutationObserverInterestGroup::createForAttributesMutation(target, attributeName);
mutationRecipients->enqueueMutationRecord(MutationRecord::createAttributes(target, attributeName, oldValue));
}
#endif
@@ -646,7 +646,8 @@
#if ENABLE(MUTATION_OBSERVERS)
// The call to attributeChanged below may dispatch DOMSubtreeModified, so it's important to enqueue a MutationRecord now.
- enqueueAttributesMutationRecord(this, attributeName, old ? old->value() : nullAtom);
+ if (!isSynchronizingStyleAttribute())
+ enqueueAttributesMutationRecord(this, attributeName, old ? old->value() : nullAtom);
#endif
if (isIdAttributeName(old ? old->name() : attributeName))
@@ -684,7 +685,8 @@
#if ENABLE(MUTATION_OBSERVERS)
// The call to attributeChanged below may dispatch DOMSubtreeModified, so it's important to enqueue a MutationRecord now.
- enqueueAttributesMutationRecord(this, name, old ? old->value() : nullAtom);
+ if (!isSynchronizingStyleAttribute())
+ enqueueAttributesMutationRecord(this, name, old ? old->value() : nullAtom);
#endif
if (isIdAttributeName(name))
Modified: trunk/Source/WebCore/dom/WebKitMutationObserver.cpp (101100 => 101101)
--- trunk/Source/WebCore/dom/WebKitMutationObserver.cpp 2011-11-23 20:18:15 UTC (rev 101100)
+++ trunk/Source/WebCore/dom/WebKitMutationObserver.cpp 2011-11-23 21:40:06 UTC (rev 101101)
@@ -40,6 +40,7 @@
#include "MutationObserverRegistration.h"
#include "MutationRecord.h"
#include "Node.h"
+#include "QualifiedName.h"
#include <wtf/ListHashSet.h>
namespace WebCore {
@@ -151,9 +152,9 @@
return adoptPtr(new MutationObserverInterestGroup(target, WebKitMutationObserver::CharacterData));
}
-PassOwnPtr<MutationObserverInterestGroup> MutationObserverInterestGroup::createForAttributesMutation(Node* target, const AtomicString& attributeName)
+PassOwnPtr<MutationObserverInterestGroup> MutationObserverInterestGroup::createForAttributesMutation(Node* target, const QualifiedName& attributeName)
{
- return adoptPtr(new MutationObserverInterestGroup(target, WebKitMutationObserver::Attributes, attributeName));
+ return adoptPtr(new MutationObserverInterestGroup(target, WebKitMutationObserver::Attributes, attributeName.localName()));
}
MutationObserverInterestGroup::MutationObserverInterestGroup(Node* target, WebKitMutationObserver::MutationType type, const AtomicString& attributeName)
Modified: trunk/Source/WebCore/dom/WebKitMutationObserver.h (101100 => 101101)
--- trunk/Source/WebCore/dom/WebKitMutationObserver.h 2011-11-23 20:18:15 UTC (rev 101100)
+++ trunk/Source/WebCore/dom/WebKitMutationObserver.h 2011-11-23 21:40:06 UTC (rev 101101)
@@ -47,6 +47,7 @@
class MutationObserverRegistration;
class MutationRecord;
class Node;
+class QualifiedName;
typedef int ExceptionCode;
@@ -99,7 +100,7 @@
public:
static PassOwnPtr<MutationObserverInterestGroup> createForChildListMutation(Node* target);
static PassOwnPtr<MutationObserverInterestGroup> createForCharacterDataMutation(Node* target);
- static PassOwnPtr<MutationObserverInterestGroup> createForAttributesMutation(Node* target, const AtomicString& attributeName);
+ static PassOwnPtr<MutationObserverInterestGroup> createForAttributesMutation(Node* target, const QualifiedName& attributeName);
bool isOldValueRequested();
bool isEmpty() { return m_observers.isEmpty(); }
