AttributeChanged can only happen for Elements, so we should pass those to it.

Comment on attachment 462852 [details] [diff] [review] Patch a: nsIMutationObserver::AttributeChanged v1 General comment: I'd prefer that we rename the argument to aElement in consumers... Calling it aContent when it's an Element is a little silly. If some of these would be nontrivial patches, then followup patches is good instead of rolling it into this one, though. >+++ b/content/base/public/nsIMutationObserver.h >-#ifndef nsIMutationObserver_h___ >-#define nsIMutationObserver_h___ Why this change? >-{ 0x85eea794, 0xed8e, 0x4e1b, \ >- { 0xa1, 0x28, 0xd0, 0x93, 0x00, 0xae, 0x51, 0xaa } } >+{ 0xe49a4a59, 0xb302, 0x4ca3, \ >+ { 0xac, 0x2c, 0xd7, 0xd0, 0xef, 0x6f, 0x25, 0x39 } } No need for that; this is a binary-compatible change at the moment. >+++ b/content/base/src/nsContentList.cpp >+nsContentList::AttributeChanged(nsIDocument *aDocument, Element* aContent, Again, aElement; this applies to all the consumers. > NS_PRECONDITION(aContent->IsElement(), "Should be an element"); This assertion can obviously go away. Was there a reason to not also patch AttributeWillChange? If no, please fix it too.

And nsCSSFrameConstructor::AttributeChanged/WillChange need fixing too, no?

Comment on attachment 462854 [details] [diff] [review] Part b: nsNodeUtils::AttributeChanged v1 r=bzbarsky

(In reply to comment #3) > Comment on attachment 462852 [details] [diff] [review] > Patch a: nsIMutationObserver::AttributeChanged v1 > > General comment: I'd prefer that we rename the argument to aElement in > consumers... Calling it aContent when it's an Element is a little silly. > > If some of these would be nontrivial patches, then followup patches is good > instead of rolling it into this one, though. OK. I wasn't sure about taking blame for those lines, but you're right. > >+++ b/content/base/public/nsIMutationObserver.h > >-#ifndef nsIMutationObserver_h___ > >-#define nsIMutationObserver_h___ > > Why this change? As David Baron said in bug 514661 comment 25, all names with two consecutive underscores are reserved to the implementation. (Though he suggests a single trailing underscore, I'm not sure why.) I'll leave it for another bug, if you prefer. > >-{ 0x85eea794, 0xed8e, 0x4e1b, \ > >- { 0xa1, 0x28, 0xd0, 0x93, 0x00, 0xae, 0x51, 0xaa } } > >+{ 0xe49a4a59, 0xb302, 0x4ca3, \ > >+ { 0xac, 0x2c, 0xd7, 0xd0, 0xef, 0x6f, 0x25, 0x39 } } > > No need for that; this is a binary-compatible change at the moment. OK. > >+++ b/content/base/src/nsContentList.cpp > >+nsContentList::AttributeChanged(nsIDocument *aDocument, Element* aContent, > > Again, aElement; this applies to all the consumers. > > > NS_PRECONDITION(aContent->IsElement(), "Should be an element"); > > This assertion can obviously go away. Done. > Was there a reason to not also patch AttributeWillChange? If no, please fix it > too. No, I'll write a patch, probably tomorrow. (In reply to comment #4) > And nsCSSFrameConstructor::AttributeChanged/WillChange need fixing too, no? Fixed.

Comment on attachment 462890 [details] [diff] [review] Part a: nsIMutationObserver::AttributeChanged v2 > +++ b/layout/base/nsCSSFrameConstructor.h >+ void AttributeChanged(mozilla::dom::Element* aElement, That can just be |Element*|, without the prefix (see the typedef in this class). r=me with that.

Comment on attachment 463479 [details] [diff] [review] Part c: AttributeWillChange v1 r=bzbarsky. Followup on the css declaration thing?

part a doesn't seem to apply cleanly anymore. patching file layout/xul/base/src/tree/src/nsTreeContentView.cpp Hunk #1 FAILED at 41

Let's try this again.

next failure in part b: patching file parser/html/nsHtml5TreeOperation.cpp Hunk #1 FAILED at 54 1 out of 2 hunks FAILED -- saving rejects to file parser/html/nsHtml5TreeOperation.cpp.rej

These really should apply.

landed and backed out, failed to compile on OS X

This one includes the bustage fix.

patching file layout/svg/base/src/nsSVGEffects.cpp Hunk #1 FAILED at 166 1 out of 1 hunks FAILED -- saving rejects to file layout/svg/base/src/nsSVGEffects.cpp.rej

Parts b and c still apply fine. Thanks a lot for your patience with me, Dão.

http://hg.mozilla.org/mozilla-central/rev/9933c41be195 http://hg.mozilla.org/mozilla-central/rev/715fc9643c0c http://hg.mozilla.org/mozilla-central/rev/f203095c85de