I noticed this when style-property-not-on-script-element-01.svg started to fail when I fixed bug 344258. An extra flush before setting the fill attribute fixes the testcase. (testcase works on Opera and Safari)

(In reply to comment #0) > An extra flush before setting the fill attribute fixes the testcase. Um, or maybe not. It was some variant of the testcase it fixed.

Er, not 344258, but 514487

Adding the .getElementById("") call flushes.

It's not so much about flushing but about when this runs isn't it? If make the change from the svg onload as a function then it works. If you run it before onload has happened (as here) then layout has not taken place and it doesn't work.

It has something to do with flush because attachment 399108 [details] works, but attachment 399105 [details] doesn't. The only difference is the document.getElementById("foo"); which flushes.

So the problem is that the script is running while the sink has unflushed content in it (!). It sets the attribute; nsSVGElement::BeforeSetAttr clears mContentStyleRule, but then under SetAttrAndNotify we end up flushing the sink: #0 nsSVGElement::WalkContentStyleRules (this=0x16d95200, aRuleWalker=0x1d5b33f0) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/svg/content/src/nsSVGElement.cpp:733 #1 0x11dbb6f2 in nsHTMLStyleSheet::RulesMatching (this=0x1e4a1ce0, aData=0xbfffab3c) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/style/nsHTMLStyleSheet.cpp:497 #2 0x11de33ad in EnumRulesMatching (aProcessor=0x1e4a1ce4, aData=0xbfffab3c) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/style/nsStyleSet.cpp:427 #3 0x11de4fd4 in nsStyleSet::FileRules (this=0x16d9e7d0, aCollectorFunc=0x11de3388 <EnumRulesMatching(nsIStyleRuleProcessor*, void*)>, aData=0xbfffab3c) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/style/nsStyleSet.cpp:554 #4 0x11de6119 in nsStyleSet::ResolveStyleFor (this=0x16d9e7d0, aContent=0x16d95200, aParentContext=0x2173dba8) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/style/nsStyleSet.cpp:693 #5 0x11ba6700 in nsCSSFrameConstructor::ResolveStyleContext (this=0x16d86ce0, aParentStyleContext=0x2173dba8, aContent=0x16d95200) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/base/nsCSSFrameConstructor.cpp:4722 #6 0x11ba6815 in nsCSSFrameConstructor::ResolveStyleContext (this=0x16d86ce0, aParentFrame=0x2173dce0, aContent=0x16d95200) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/base/nsCSSFrameConstructor.cpp:4712 #7 0x11bab3f7 in nsCSSFrameConstructor::AddFrameConstructionItems (this=0x16d86ce0, aState=@0xbfffacd8, aContent=0x16d95200, aContentIndex=7, aParentFrame=0x2173dce0, aItems=@0xbfffad24) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/base/nsCSSFrameConstructor.cpp:5164 #8 0x11bbc40d in nsCSSFrameConstructor::ContentAppended (this=0x16d86ce0, aContainer=0x16d6f760, aNewIndexInContainer=6) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/base/nsCSSFrameConstructor.cpp:6365 #9 0x11c3174b in PresShell::ContentAppended (this=0x16d9e890, aDocument=0x1500a00, aContainer=0x16d6f760, aNewIndexInContainer=6) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/base/nsPresShell.cpp:4894 #10 0x11f2e4b0 in nsNodeUtils::ContentAppended (aContainer=0x16d6f760, aNewIndexInContainer=6) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsNodeUtils.cpp:132 #11 0x11e95560 in nsContentSink::NotifyAppend (this=0x16d921d0, aContainer=0x16d6f760, aStartIndex=6) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsContentSink.cpp:1375 #12 0x120815e8 in nsXMLContentSink::FlushTags (this=0x16d921d0) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/xml/document/src/nsXMLContentSink.cpp:1680 #13 0x11e946d9 in nsContentSink::BeginUpdate (this=0x16d921d0, aDocument=0x1500a00, aUpdateType=1) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsContentSink.cpp:1612 #14 0x11edcbf1 in nsDocument::BeginUpdate (this=0x1500a00, aUpdateType=1) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsDocument.cpp:3748 #15 0x11c92593 in mozAutoDocUpdate::mozAutoDocUpdate (this=0xbfffb214, aDocument=0x1500a00, aUpdateType=1, aNotify=1) at mozAutoDocUpdate.h:53 #16 0x11f11460 in nsGenericElement::SetAttrAndNotify (this=0x16d95090, aNamespaceID=0, aName=0x121b634, aPrefix=0x0, aOldValue=@0xbfffb2a4, aParsedValue=@0xbfffb340, aModification=1, aFireMutation=0, aNotify=1, aValueForAfterSetAttr=0xbfffb43c) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsGenericElement.cpp:4386 #17 0x11f1275f in nsGenericElement::SetAttr (this=0x16d95090, aNamespaceID=0, aName=0x121b634, aPrefix=0x0, aValue=@0xbfffb43c, aNotify=1) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsGenericElement.cpp:4366 This causes us to resolve style for the <rect>, which uses the _old_ attribute value for creating the mContentStyleRule (since the new one hasn't been set yet). Then we never create an mContentStyleRule with the new value. It's weird that we don't flush the sink before running the script. It's a bit weird to me that we don't start the attr-set update before we call BeforeSetAttr, maybe. But given that we don't, clearing the mContentStyleRule in BeforeSetAttr is just wrong... The right place to do it is in AfterSetAttr, I would think.

Forgot to fix up the reftest title I'll make change it to this on check in. <title>Testcase ensuring fill works if applied before onload</title>

pushed http://hg.mozilla.org/mozilla-central/rev/7d03ad8e0e5b

Comment on attachment 400003 [details] [diff] [review] patch Simple fix with testcase for longstanding issue

pushed http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ecda7818bb68