Bug 1149042 - Call AttributeWillChange before a style="" attribute gets created when touching element.style. r=smaug

This commit is contained in:
Cameron McCormack 2015-04-10 10:41:35 +10:00
parent 74ece90249
commit 55844d9667
10 changed files with 150 additions and 38 deletions

View File

@ -580,7 +580,7 @@ function testExpandos() {
var m2 = new M(function(records, observer) {
is(observer.expandoProperty, true);
observer.disconnect();
then();
then(testStyleCreate);
});
m2.expandoProperty = true;
m2.observe(div, { attributes: true });
@ -596,6 +596,91 @@ function testExpandos() {
div.setAttribute("foo", "bar2");
}
function testStyleCreate() {
m = new M(function(records, observer) {
is(records.length, 1, "number of records");
is(records[0].type, "attributes", "record.type");
is(records[0].attributeName, "style", "record.attributeName");
is(records[0].oldValue, null, "record.oldValue");
isnot(div.getAttribute("style"), null, "style attribute after creation");
observer.disconnect();
m = null;
div.removeAttribute("style");
then(testStyleModify);
});
m.observe(div, { attributes: true, attributeOldValue: true });
is(div.getAttribute("style"), null, "style attribute before creation");
div.style.color = "blue";
}
function testStyleModify() {
div.style.color = "yellow";
m = new M(function(records, observer) {
is(records.length, 1, "number of records");
is(records[0].type, "attributes", "record.type");
is(records[0].attributeName, "style", "record.attributeName");
isnot(div.getAttribute("style"), null, "style attribute after modification");
observer.disconnect();
m = null;
div.removeAttribute("style");
then(testStyleRead);
});
m.observe(div, { attributes: true });
isnot(div.getAttribute("style"), null, "style attribute before modification");
div.style.color = "blue";
}
function testStyleRead() {
m = new M(function(records, observer) {
is(records.length, 1, "number of records");
is(records[0].type, "attributes", "record.type");
is(records[0].attributeName, "data-test", "record.attributeName");
is(div.getAttribute("style"), null, "style attribute after read");
observer.disconnect();
div.removeAttribute("data-test");
m = null;
then(testStyleRemoveProperty);
});
m.observe(div, { attributes: true });
is(div.getAttribute("style"), null, "style attribute before read");
var value = div.style.color; // shouldn't generate any mutation records
div.setAttribute("data-test", "a");
}
function testStyleRemoveProperty() {
div.style.color = "blue";
m = new M(function(records, observer) {
is(records.length, 1, "number of records");
is(records[0].type, "attributes", "record.type");
is(records[0].attributeName, "style", "record.attributeName");
isnot(div.getAttribute("style"), null, "style attribute after successful removeProperty");
observer.disconnect();
m = null;
div.removeAttribute("style");
then(testStyleRemoveProperty2);
});
m.observe(div, { attributes: true });
isnot(div.getAttribute("style"), null, "style attribute before successful removeProperty");
div.style.removeProperty("color");
}
function testStyleRemoveProperty2() {
m = new M(function(records, observer) {
is(records.length, 1, "number of records");
is(records[0].type, "attributes", "record.type");
is(records[0].attributeName, "data-test", "record.attributeName");
is(div.getAttribute("style"), null, "style attribute after unsuccessful removeProperty");
observer.disconnect();
m = null;
div.removeAttribute("data-test");
then();
});
m.observe(div, { attributes: true });
is(div.getAttribute("style"), null, "style attribute before unsuccessful removeProperty");
div.style.removeProperty("color"); // shouldn't generate any mutation records
div.setAttribute("data-test", "a");
}
SimpleTest.waitForExplicitFinish();
</script>

View File

@ -1009,7 +1009,7 @@ public:
NS_IMETHOD GetParentRule(nsIDOMCSSRule **aParent) override;
void DropReference(void);
virtual css::Declaration* GetCSSDeclaration(bool aAllocate) override;
virtual css::Declaration* GetCSSDeclaration(Operation aOperation) override;
virtual nsresult SetCSSDeclaration(css::Declaration* aDecl) override;
virtual void GetCSSParsingEnvironment(CSSParsingEnvironment& aCSSParseEnv) override;
virtual nsIDocument* DocToUpdate() override;
@ -1114,7 +1114,7 @@ DOMCSSDeclarationImpl::DropReference(void)
}
css::Declaration*
DOMCSSDeclarationImpl::GetCSSDeclaration(bool aAllocate)
DOMCSSDeclarationImpl::GetCSSDeclaration(Operation aOperation)
{
if (mRule) {
return mRule->GetDeclaration();

View File

@ -2103,7 +2103,7 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsCSSKeyframeStyleDeclaration)
NS_INTERFACE_MAP_END_INHERITING(nsDOMCSSDeclaration)
css::Declaration*
nsCSSKeyframeStyleDeclaration::GetCSSDeclaration(bool aAllocate)
nsCSSKeyframeStyleDeclaration::GetCSSDeclaration(Operation aOperation)
{
if (mRule) {
return mRule->Declaration();
@ -2664,7 +2664,7 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsCSSPageStyleDeclaration)
NS_INTERFACE_MAP_END_INHERITING(nsDOMCSSDeclaration)
css::Declaration*
nsCSSPageStyleDeclaration::GetCSSDeclaration(bool aAllocate)
nsCSSPageStyleDeclaration::GetCSSDeclaration(Operation aOperation)
{
if (mRule) {
return mRule->Declaration();

View File

@ -408,7 +408,7 @@ public:
NS_IMETHOD GetParentRule(nsIDOMCSSRule **aParent) override;
void DropReference() { mRule = nullptr; }
virtual mozilla::css::Declaration* GetCSSDeclaration(bool aAllocate) override;
virtual mozilla::css::Declaration* GetCSSDeclaration(Operation aOperation) override;
virtual nsresult SetCSSDeclaration(mozilla::css::Declaration* aDecl) override;
virtual void GetCSSParsingEnvironment(CSSParsingEnvironment& aCSSParseEnv) override;
virtual nsIDocument* DocToUpdate() override;
@ -541,7 +541,7 @@ public:
NS_IMETHOD GetParentRule(nsIDOMCSSRule **aParent) override;
void DropReference() { mRule = nullptr; }
virtual mozilla::css::Declaration* GetCSSDeclaration(bool aAllocate) override;
virtual mozilla::css::Declaration* GetCSSDeclaration(Operation aOperation) override;
virtual nsresult SetCSSDeclaration(mozilla::css::Declaration* aDecl) override;
virtual void GetCSSParsingEnvironment(CSSParsingEnvironment& aCSSParseEnv) override;
virtual nsIDocument* DocToUpdate() override;

View File

@ -543,7 +543,7 @@ nsComputedDOMStyle::GetPresShellForContent(nsIContent* aContent)
// on a nsComputedDOMStyle object, but must be defined to avoid
// compile errors.
css::Declaration*
nsComputedDOMStyle::GetCSSDeclaration(bool)
nsComputedDOMStyle::GetCSSDeclaration(Operation)
{
NS_RUNTIMEABORT("called nsComputedDOMStyle::GetCSSDeclaration");
return nullptr;

View File

@ -120,7 +120,7 @@ public:
// nsDOMCSSDeclaration abstract methods which should never be called
// on a nsComputedDOMStyle object, but must be defined to avoid
// compile errors.
virtual mozilla::css::Declaration* GetCSSDeclaration(bool) override;
virtual mozilla::css::Declaration* GetCSSDeclaration(Operation) override;
virtual nsresult SetCSSDeclaration(mozilla::css::Declaration*) override;
virtual nsIDocument* DocToUpdate() override;
virtual void GetCSSParsingEnvironment(CSSParsingEnvironment& aCSSParseEnv) override;

View File

@ -91,23 +91,13 @@ nsDOMCSSAttributeDeclaration::SetCSSDeclaration(css::Declaration* aDecl)
nsIDocument*
nsDOMCSSAttributeDeclaration::DocToUpdate()
{
// XXXbz this is a bit of a hack, especially doing it before the
// BeginUpdate(), but this is a good chokepoint where we know we
// plan to modify the CSSDeclaration, so need to notify
// AttributeWillChange if this is inline style.
if (!mIsSMILOverride) {
nsNodeUtils::AttributeWillChange(mElement, kNameSpaceID_None,
nsGkAtoms::style,
nsIDOMMutationEvent::MODIFICATION);
}
// We need OwnerDoc() rather than GetCurrentDoc() because it might
// be the BeginUpdate call that inserts mElement into the document.
return mElement->OwnerDoc();
}
css::Declaration*
nsDOMCSSAttributeDeclaration::GetCSSDeclaration(bool aAllocate)
nsDOMCSSAttributeDeclaration::GetCSSDeclaration(Operation aOperation)
{
if (!mElement)
return nullptr;
@ -118,10 +108,31 @@ nsDOMCSSAttributeDeclaration::GetCSSDeclaration(bool aAllocate)
else
cssRule = mElement->GetInlineStyleRule();
// Notify observers that our style="" attribute is going to change
// unless:
// * this is a declaration that holds SMIL animation values (which
// aren't reflected in the DOM style="" attribute), or
// * we're getting the declaration for reading, or
// * we're getting it for property removal but we don't currently have
// a declaration.
// XXXbz this is a bit of a hack, especially doing it before the
// BeginUpdate(), but this is a good chokepoint where we know we
// plan to modify the CSSDeclaration, so need to notify
// AttributeWillChange if this is inline style.
if (!mIsSMILOverride &&
((aOperation == eOperation_Modify) ||
(aOperation == eOperation_RemoveProperty && cssRule))) {
nsNodeUtils::AttributeWillChange(mElement, kNameSpaceID_None,
nsGkAtoms::style,
nsIDOMMutationEvent::MODIFICATION);
}
if (cssRule) {
return cssRule->GetDeclaration();
}
if (!aAllocate) {
if (aOperation != eOperation_Modify) {
return nullptr;
}

View File

@ -31,7 +31,7 @@ public:
// If GetCSSDeclaration returns non-null, then the decl it returns
// is owned by our current style rule.
virtual mozilla::css::Declaration* GetCSSDeclaration(bool aAllocate) override;
virtual mozilla::css::Declaration* GetCSSDeclaration(Operation aOperation) override;
virtual void GetCSSParsingEnvironment(CSSParsingEnvironment& aCSSParseEnv) override;
NS_IMETHOD GetParentRule(nsIDOMCSSRule **aParent) override;

View File

@ -45,7 +45,7 @@ nsDOMCSSDeclaration::GetPropertyValue(const nsCSSProperty aPropID,
NS_PRECONDITION(aPropID != eCSSProperty_UNKNOWN,
"Should never pass eCSSProperty_UNKNOWN around");
css::Declaration* decl = GetCSSDeclaration(false);
css::Declaration* decl = GetCSSDeclaration(eOperation_Read);
aValue.Truncate();
if (decl) {
@ -61,7 +61,7 @@ nsDOMCSSDeclaration::GetCustomPropertyValue(const nsAString& aPropertyName,
MOZ_ASSERT(Substring(aPropertyName, 0,
CSS_CUSTOM_NAME_PREFIX_LENGTH).EqualsLiteral("--"));
css::Declaration* decl = GetCSSDeclaration(false);
css::Declaration* decl = GetCSSDeclaration(eOperation_Read);
if (!decl) {
aValue.Truncate();
return;
@ -89,7 +89,7 @@ nsDOMCSSDeclaration::SetPropertyValue(const nsCSSProperty aPropID,
NS_IMETHODIMP
nsDOMCSSDeclaration::GetCssText(nsAString& aCssText)
{
css::Declaration* decl = GetCSSDeclaration(false);
css::Declaration* decl = GetCSSDeclaration(eOperation_Read);
aCssText.Truncate();
if (decl) {
@ -104,7 +104,7 @@ nsDOMCSSDeclaration::SetCssText(const nsAString& aCssText)
{
// We don't need to *do* anything with the old declaration, but we need
// to ensure that it exists, or else SetCSSDeclaration may crash.
css::Declaration* olddecl = GetCSSDeclaration(true);
css::Declaration* olddecl = GetCSSDeclaration(eOperation_Modify);
if (!olddecl) {
return NS_ERROR_FAILURE;
}
@ -139,7 +139,7 @@ nsDOMCSSDeclaration::SetCssText(const nsAString& aCssText)
NS_IMETHODIMP
nsDOMCSSDeclaration::GetLength(uint32_t* aLength)
{
css::Declaration* decl = GetCSSDeclaration(false);
css::Declaration* decl = GetCSSDeclaration(eOperation_Read);
if (decl) {
*aLength = decl->Count();
@ -161,7 +161,7 @@ nsDOMCSSDeclaration::GetPropertyCSSValue(const nsAString& aPropertyName, ErrorRe
void
nsDOMCSSDeclaration::IndexedGetter(uint32_t aIndex, bool& aFound, nsAString& aPropName)
{
css::Declaration* decl = GetCSSDeclaration(false);
css::Declaration* decl = GetCSSDeclaration(eOperation_Read);
aFound = decl && decl->GetNthProperty(aIndex, aPropName);
}
@ -202,7 +202,7 @@ nsDOMCSSDeclaration::GetAuthoredPropertyValue(const nsAString& aPropertyName,
return NS_OK;
}
css::Declaration* decl = GetCSSDeclaration(false);
css::Declaration* decl = GetCSSDeclaration(eOperation_Read);
if (!decl) {
return NS_ERROR_FAILURE;
}
@ -215,7 +215,7 @@ NS_IMETHODIMP
nsDOMCSSDeclaration::GetPropertyPriority(const nsAString& aPropertyName,
nsAString& aReturn)
{
css::Declaration* decl = GetCSSDeclaration(false);
css::Declaration* decl = GetCSSDeclaration(eOperation_Read);
aReturn.Truncate();
if (decl && decl->GetValueIsImportant(aPropertyName)) {
@ -310,7 +310,7 @@ nsDOMCSSDeclaration::ParsePropertyValue(const nsCSSProperty aPropID,
const nsAString& aPropValue,
bool aIsImportant)
{
css::Declaration* olddecl = GetCSSDeclaration(true);
css::Declaration* olddecl = GetCSSDeclaration(eOperation_Modify);
if (!olddecl) {
return NS_ERROR_FAILURE;
}
@ -351,7 +351,7 @@ nsDOMCSSDeclaration::ParseCustomPropertyValue(const nsAString& aPropertyName,
{
MOZ_ASSERT(nsCSSProps::IsCustomPropertyName(aPropertyName));
css::Declaration* olddecl = GetCSSDeclaration(true);
css::Declaration* olddecl = GetCSSDeclaration(eOperation_Modify);
if (!olddecl) {
return NS_ERROR_FAILURE;
}
@ -391,7 +391,7 @@ nsDOMCSSDeclaration::ParseCustomPropertyValue(const nsAString& aPropertyName,
nsresult
nsDOMCSSDeclaration::RemoveProperty(const nsCSSProperty aPropID)
{
css::Declaration* decl = GetCSSDeclaration(false);
css::Declaration* decl = GetCSSDeclaration(eOperation_RemoveProperty);
if (!decl) {
return NS_OK; // no decl, so nothing to remove
}
@ -414,7 +414,7 @@ nsDOMCSSDeclaration::RemoveCustomProperty(const nsAString& aPropertyName)
MOZ_ASSERT(Substring(aPropertyName, 0,
CSS_CUSTOM_NAME_PREFIX_LENGTH).EqualsLiteral("--"));
css::Declaration* decl = GetCSSDeclaration(false);
css::Declaration* decl = GetCSSDeclaration(eOperation_RemoveProperty);
if (!decl) {
return NS_OK; // no decl, so nothing to remove
}

View File

@ -100,10 +100,26 @@ public:
virtual JSObject* WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
protected:
// This method can return null regardless of the value of aAllocate;
// however, a null return should only be considered a failure
// if aAllocate is true.
virtual mozilla::css::Declaration* GetCSSDeclaration(bool aAllocate) = 0;
// The reason for calling GetCSSDeclaration.
enum Operation {
// We are calling GetCSSDeclaration so that we can read from it. Does not
// allocate a new declaration if we don't have one yet; returns nullptr in
// this case.
eOperation_Read,
// We are calling GetCSSDeclaration so that we can set a property on it
// or re-parse the whole declaration. Allocates a new declaration if we
// don't have one yet and calls AttributeWillChange. A nullptr return value
// indicates an error allocating the declaration.
eOperation_Modify,
// We are calling GetCSSDeclaration so that we can remove a property from
// it. Does not allocates a new declaration if we don't have one yet;
// returns nullptr in this case. If we do have a declaration, calls
// AttributeWillChange.
eOperation_RemoveProperty
};
virtual mozilla::css::Declaration* GetCSSDeclaration(Operation aOperation) = 0;
virtual nsresult SetCSSDeclaration(mozilla::css::Declaration* aDecl) = 0;
// Document that we must call BeginUpdate/EndUpdate on around the
// calls to SetCSSDeclaration and the style rule mutation that leads