Review assigned to smaug by a fair dice roll.

I think you should nuke the pref and the check for quirks mode, since the spec doesn't have any of that stuff....

There's something rotten... nsDocument::Destroy calls nsContentUtils::ReleaseWrapper, which ends up in XPCJSRuntime::RemoveJSHolder. XPCJSRuntime::RemoveJSHolder then tries to assert that there's nothing to trace, but there actually is something to trace: the mAll member. Any hints on how to get around this?

Other than moving the "all" object itself to WebIDL?

Khuey suggested clearing mAll in Destroy; that seems a little less invasive.

(In reply to Boris Zbarsky (:bz) from comment #1) > I think you should nuke the pref and the check for quirks mode, since the > spec doesn't have any of that stuff.... +1 @Ms2ger: Please update the quirks-mode-spec if you insist on checking the quirks mode.

Comment on attachment 757124 [details] [diff] [review] Patch v1.1 >@@ -25,9 +25,10 @@ DEPRECATED_OPERATION(DOMExceptionCode) > DEPRECATED_OPERATION(NoExposedProps) > DEPRECATED_OPERATION(MutationEvent) > DEPRECATED_OPERATION(MozSlice) > DEPRECATED_OPERATION(Components) > DEPRECATED_OPERATION(PrefixedVisibilityAPI) > DEPRECATED_OPERATION(NodeIteratorDetach) > DEPRECATED_OPERATION(MozAudioData) > DEPRECATED_OPERATION(LenientThis) >+DEPRECATED_OPERATION(DocumentAll) I'd prefer to not add this (and just remove the existing warning) >+nsHTMLDocument::~nsHTMLDocument() >+{ >+ NS_DROP_JS_OBJECTS(this, nsHTMLDocument); You should set mAll null before drop, otherwise an assertion may fire. (The assertion checks that there isn't anything to trace when drop is called.) >+nsHTMLDocument::ExposeDocumentAll(JSContext* aCx, JSObject* aGlobal) Could we just not have this stuff. >+JSObject* >+nsHTMLDocument::GetAll(JSContext* aCx, ErrorResult& aRv) >+{ >+ WarnOnceAbout(eDocumentAll); >+ if (!mAll) { >+ mAll = JS_NewObject(aCx, &sHTMLDocumentAllClass, nullptr, >+ JS_GetGlobalForObject(aCx, GetWrapper())); >+ if (!mAll) { >+ aRv.Throw(NS_ERROR_OUT_OF_MEMORY); >+ return nullptr; >+ } >+ >+ JS_SetPrivate(mAll, static_cast<nsINode*>(this)); >+ NS_ADDREF_THIS(); Could you copy the comment from the old code. // Let the JSObject take over ownership of doc.

Comment on attachment 757485 [details] [diff] [review] Patch v2 >+nsHTMLDocument::~nsHTMLDocument() >+{ >+ NS_DROP_JS_OBJECTS(this, nsHTMLDocument); Please add mAll = nullptr; before drop. And crossing fingers :)