Comment on attachment 8399872 [details] [diff] [review] v1 > +nsGlobalWindow::GetDialogArguments(JSContext* aCx, ErrorResult& aError) Do we not need to enter the compartment of GetWrapper() before calling mDialogArguments->Get()? I think we should, or else we need a nice comment explaining why we're not doing it. >+nsGlobalWindow::GetReturnValue(JSContext* aCx, ErrorResult& aError) Same here, for mReturnValue->Get(). >+nsGlobalWindow::SetReturnValue(JSContext* aCx, >+ FORWARD_TO_OUTER_OR_THROW(SetReturnValue, (aCx, aReturnValue, aError), aError, >+ ); The linebreak there is up. Please either hoist the ");" or drop the aError to the next line? >+++ b/dom/base/nsGlobalWindow.h >+ aResult.set(JS::UndefinedValue()); aResult.setUndefined(); >+ nsRefPtr<DialogValueHolder> mReturnValue; Document that this is outer-window-only? >+++ b/dom/webidl/Window.webidl Could we put the new interface above the mozilla-proprietary bits like sidebar? r=me with that.

Had to remove the dom/base/test/test_openDialogChromeOnly.html fix from this patch, it relies on the new bindings :-/. https://hg.mozilla.org/integration/mozilla-inbound/rev/4fbe5c6db773 and a followup rooting fix: https://hg.mozilla.org/integration/mozilla-inbound/rev/72b4d4463629

https://hg.mozilla.org/mozilla-central/rev/4fbe5c6db773 https://hg.mozilla.org/mozilla-central/rev/72b4d4463629