Bug 1340341 - Set immutablity in GetCSSDeclaration instead of ToString. r?dbaron
MozReview-Commit-ID: 2aDsVJMJG8T
--- a/layout/style/Declaration.cpp
+++ b/layout/style/Declaration.cpp
@@ -1696,20 +1696,16 @@ Declaration::AppendVariableAndValueToStr
aResult.AppendLiteral("!important");
}
aResult.AppendLiteral("; ");
}
void
Declaration::ToString(nsAString& aString) const
{
- // Someone cares about this declaration's contents, so don't let it
- // change from under them. See e.g. bug 338679.
- SetImmutable();
-
nsCSSCompressedDataBlock *systemFontData =
GetPropertyIsImportantByID(eCSSProperty__x_system_font) ? mImportantData
: mData;
const nsCSSValue *systemFont =
systemFontData->ValueFor(eCSSProperty__x_system_font);
const bool haveSystemFont = systemFont &&
systemFont->GetUnit() != eCSSUnit_None &&
systemFont->GetUnit() != eCSSUnit_Null;
--- a/layout/style/DeclarationBlock.h
+++ b/layout/style/DeclarationBlock.h
@@ -58,17 +58,17 @@ public:
void AssertMutable() const {
MOZ_ASSERT(IsMutable(), "someone forgot to call EnsureMutable");
}
/**
* Mark this declaration as unmodifiable. It's 'const' so it can
* be called from ToString.
*/
- void SetImmutable() const { mImmutable = true; }
+ void SetImmutable() { mImmutable = true; }
/**
* Copy |this|, if necessary to ensure that it can be modified.
*/
inline already_AddRefed<DeclarationBlock> EnsureMutable();
void SetOwningRule(css::Rule* aRule) {
MOZ_ASSERT(!mContainer.mOwningRule || !aRule,
@@ -130,17 +130,16 @@ private:
css::Rule* mOwningRule;
// The nsHTMLCSSStyleSheet that is responsible for this declaration.
// Only non-null for style attributes.
nsHTMLCSSStyleSheet* mHTMLCSSStyleSheet;
} mContainer;
// set when declaration put in the rule tree;
- // also by ToString (hence the 'mutable').
- mutable bool mImmutable;
+ bool mImmutable;
const StyleBackendType mType;
};
} // namespace mozilla
#endif // mozilla_DeclarationBlock_h
--- a/layout/style/nsDOMCSSAttrDeclaration.cpp
+++ b/layout/style/nsDOMCSSAttrDeclaration.cpp
@@ -7,17 +7,19 @@
#include "nsDOMCSSAttrDeclaration.h"
#include "mozilla/css/Declaration.h"
#include "mozilla/css/StyleRule.h"
#include "mozilla/DeclarationBlock.h"
#include "mozilla/DeclarationBlockInlines.h"
#include "mozilla/dom/Element.h"
+#include "mozilla/InternalMutationEvent.h"
#include "mozilla/ServoDeclarationBlock.h"
+#include "nsContentUtils.h"
#include "nsIDocument.h"
#include "nsIDOMMutationEvent.h"
#include "nsIURI.h"
#include "nsNodeUtils.h"
#include "nsWrapperCacheInlines.h"
#include "nsIFrame.h"
#include "ActiveLayerTracker.h"
@@ -114,16 +116,25 @@ nsDOMCSSAttributeDeclaration::GetCSSDecl
(aOperation == eOperation_RemoveProperty && declaration))) {
nsNodeUtils::AttributeWillChange(mElement, kNameSpaceID_None,
nsGkAtoms::style,
nsIDOMMutationEvent::MODIFICATION,
nullptr);
}
if (declaration) {
+ if (aOperation != eOperation_Read &&
+ nsContentUtils::HasMutationListeners(
+ mElement, NS_EVENT_BITS_MUTATION_ATTRMODIFIED, mElement)) {
+ // If there is any mutation listener on the element, we need to
+ // ensure that any change would create a new declaration so that
+ // nsStyledElement::SetInlineStyleDeclaration can generate the
+ // correct old value.
+ declaration->SetImmutable();
+ }
return declaration;
}
if (aOperation != eOperation_Modify) {
return nullptr;
}
// cannot fail
--- a/layout/style/test/mochitest.ini
+++ b/layout/style/test/mochitest.ini
@@ -251,16 +251,17 @@ support-files = redundant_font_download.
[test_rule_serialization.html]
[test_rules_out_of_sheets.html]
[test_selectors.html]
skip-if = toolkit == 'android' #bug 775227
[test_selectors_on_anonymous_content.html]
[test_setPropertyWithNull.html]
[test_shorthand_property_getters.html]
[test_specified_value_serialization.html]
+[test_style_attr_listener.html]
[test_style_attribute_quirks.html]
[test_style_attribute_standards.html]
[test_style_struct_copy_constructors.html]
[test_supports_rules.html]
[test_system_font_serialization.html]
[test_text_decoration_shorthands.html]
[test_transitions_and_reframes.html]
[test_transitions_and_restyles.html]
new file mode 100644
--- /dev/null
+++ b/layout/style/test/test_style_attr_listener.html
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <title>Test for Bug 338679 (from bug 1340341)</title>
+ <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+ <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<div style=""></div>
+<script>
+let div = document.querySelector('div');
+let expectation;
+let count = 0;
+div.style.color = "red";
+div.addEventListener('DOMAttrModified', function(evt) {
+ count++;
+ is(evt.prevValue, expectation.prevValue, "Previous value for event ${count}");
+ is(evt.newValue, expectation.newValue, "New value for event ${count}");
+});
+expectation = { prevValue: 'color: red;', newValue: 'color: green;' };
+div.style.color = "green";
+expectation = { prevValue: 'color: green;', newValue: '' };
+div.style.color = '';
+</script>
+</body>
+</html>