Bug 338679: mark style declarations immutable if they are stringified. r=bzbarsky a=dbaron

This commit is contained in:
Zack Weinberg 2010-08-02 15:40:35 -07:00
parent 24feb13867
commit 19e2c4da92
4 changed files with 77 additions and 42 deletions

View File

@ -4,52 +4,76 @@
https://bugzilla.mozilla.org/show_bug.cgi?id=338679
-->
<head>
<title>Testcase bug 338679</title>
<title>Bug 338679: correct reporting of newValue/prevValue in
DOMAttrModified events</title>
<script type="text/javascript" src="/MochiKit/packed.js"></script>
<script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
<script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
</head>
<body>
<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=338679">Mozilla Bug 338679</a>
<div >
<dl>
<dt>Actual result:</dt>
<dd>
<pre id="out" style="width: 20em">
</pre>
</dd>
<a target="_blank"
href="https://bugzilla.mozilla.org/show_bug.cgi?id=338679">Bug
338679: correct reporting of newValue/prevValue in
DOMAttrModified events</a>
<dt>Expected result:</dt>
<div id="test" style="width:20em"></div>
<dd>
<pre>
Previous: width: 20em;
New: width: auto;
</pre>
</dd>
</dl>
</div>
<pre id="test">
<script>
var testDiv = document.getElementById("test");
var e_new, e_prev = testDiv.getAttribute("style");
var phase, recursive = false;
/* This is our event handler function */
function attr_modified(ev) {
$("out").textContent = "Previous:\t" + ev.prevValue + "\nNew:\t\t" + ev.newValue;
is(ev.newValue, "width: auto;", "DOMAttrModified event reports correct newValue");
is(ev.prevValue == "width: 20em;", "DOMAttrModified event reports correct prevValue");
SimpleTest.finish(); // trigger the end of our test sequence
/* driver */
var tests = [ test_1, test_2, test_3 ];
var i = 0;
function nextTest() {
if (i < tests.length) {
phase = tests[i];
i++;
phase();
} else {
SimpleTest.finish();
}
}
/* Call this to tell SimpleTest to wait for SimpleTest.finish() */
SimpleTest.waitForExplicitFinish();
testDiv.addEventListener("DOMAttrModified", attr_modified, false);
nextTest();
$("out").addEventListener("DOMAttrModified", attr_modified, false);
$("out").style.width = "auto";
/* event handler */
function attr_modified(ev) {
is(ev.newValue, e_new,
phase.name + (recursive ? " recursive" : "") + ": newValue");
is(ev.prevValue, e_prev,
phase.name + (recursive ? " recursive" : "") + ": prevValue");
/* End of script, but SimpleTest will keep listening because
we called waitForExplicitFinish() */
e_prev = e_new;
if (!recursive) {
recursive = true;
e_new = "width: 0pt;";
testDiv.style.width = "0";
} else {
recursive = false;
setTimeout(nextTest, 0);
}
}
/* tests */
function test_1() {
e_new = "width: auto;";
testDiv.style.width = "auto";
}
function test_2() {
e_new = "width: 15%;";
testDiv.style.width = "15%";
}
function test_3() {
window.getComputedStyle(testDiv, null).width; // force style resolution
e_new = "width: inherit;";
testDiv.style.width = "inherit";
}
</script>
</pre>
</body>
</html>

View File

@ -771,6 +771,10 @@ Declaration::AppendPropertyAndValueToString(nsCSSProperty aProperty,
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 =
GetValueIsImportant(eCSSProperty__x_system_font) ? mImportantData : mData;
const nsCSSValue *systemFont =

View File

@ -213,9 +213,10 @@ public:
}
/**
* Mark this declaration as unmodifiable.
* Mark this declaration as unmodifiable. It's 'const' so it can
* be called from ToString.
*/
void SetImmutable() { mImmutable = PR_TRUE; }
void SetImmutable() const { mImmutable = PR_TRUE; }
/**
* Clear the data, in preparation for its replacement with entirely
@ -258,8 +259,9 @@ private:
// may be null
nsAutoPtr<nsCSSCompressedDataBlock> mImportantData;
// set by style rules when |RuleMatched| is called
PRPackedBool mImmutable;
// set by style rules when |RuleMatched| is called;
// also by ToString (hence the 'mutable').
mutable PRPackedBool mImmutable;
};
} // namespace css

View File

@ -1336,6 +1336,7 @@ protected:
CSSImportantRule* mImportantRule; // initialized by RuleMatched
DOMCSSStyleRuleImpl* mDOMRule;
PRUint32 mLineNumber;
PRPackedBool mWasMatched;
};
CSSStyleRuleImpl::CSSStyleRuleImpl(nsCSSSelectorList* aSelector,
@ -1345,7 +1346,8 @@ CSSStyleRuleImpl::CSSStyleRuleImpl(nsCSSSelectorList* aSelector,
mDeclaration(aDeclaration),
mImportantRule(nsnull),
mDOMRule(nsnull),
mLineNumber(0)
mLineNumber(0),
mWasMatched(PR_FALSE)
{
}
@ -1356,7 +1358,8 @@ CSSStyleRuleImpl::CSSStyleRuleImpl(const CSSStyleRuleImpl& aCopy)
mDeclaration(new css::Declaration(*aCopy.mDeclaration)),
mImportantRule(nsnull),
mDOMRule(nsnull),
mLineNumber(aCopy.mLineNumber)
mLineNumber(aCopy.mLineNumber),
mWasMatched(PR_FALSE)
{
// rest is constructed lazily on existing data
}
@ -1369,7 +1372,8 @@ CSSStyleRuleImpl::CSSStyleRuleImpl(CSSStyleRuleImpl& aCopy,
mDeclaration(aDeclaration),
mImportantRule(nsnull),
mDOMRule(aCopy.mDOMRule),
mLineNumber(aCopy.mLineNumber)
mLineNumber(aCopy.mLineNumber),
mWasMatched(PR_FALSE)
{
// The DOM rule is replacing |aCopy| with |this|, so transfer
// the reverse pointer as well (and transfer ownership).
@ -1438,9 +1442,10 @@ nsIStyleRule* CSSStyleRuleImpl::GetImportantRule(void)
/* virtual */ void
CSSStyleRuleImpl::RuleMatched()
{
if (mDeclaration->IsMutable()) {
if (!mWasMatched) {
NS_ABORT_IF_FALSE(!mImportantRule, "should not have important rule yet");
mWasMatched = PR_TRUE;
mDeclaration->SetImmutable();
if (mDeclaration->HasImportantData()) {
NS_ADDREF(mImportantRule = new CSSImportantRule(mDeclaration));
@ -1532,7 +1537,7 @@ CSSStyleRuleImpl::DeclarationChanged(css::Declaration* aDecl,
/* virtual */ void
CSSStyleRuleImpl::MapRuleInfoInto(nsRuleData* aRuleData)
{
NS_ABORT_IF_FALSE(!mDeclaration->IsMutable(),
NS_ABORT_IF_FALSE(mWasMatched,
"somebody forgot to call nsICSSStyleRule::RuleMatched");
mDeclaration->MapNormalRuleInfoInto(aRuleData);
}