Note: This will affect embeddors (launcher->Cancel() calls need to be replaced with launcher->Cancel(NS_BINDING_ABORTED) or another error code) nsIHelperAppLauncher should implement nsICancelable. this is part of the work to make all the download-related interfaces implemented by mozilla code implement nsICancelable.

Comment on attachment 180333 [details] [diff] [review] patch >Index: embedding/components/ui/helperAppDlg/nsHelperAppDlg.js >+ const NS_BINDING_ABORTED = 0x80020006; >+ this.mLauncher.cancel(NS_BINDING_ABORTED); This is not NS_BINDING_ABORTED. I think you want 0x804b0002 >Index: uriloader/exthandler/nsExternalHelperAppService.cpp >+NS_IMETHODIMP nsExternalAppHandler::Cancel(nsresult aReason) > { >+ // XXX should not ignore the reason How about adding a NS_ERROR_ARG(NS_FAILED(aReason)) at least to ensure that people use this API correctly even if we aren't going to use aReason for anything yet. sr=darin with those changes

> This is not NS_BINDING_ABORTED. I think you want 0x804b0002 huh, true, I copied that from http://lxr.mozilla.org/seamonkey/source/browser/components/nsBrowserContentHandler.js#62. I filed bug 289981

justification for approval request: this is an api change, and thus it'd be nice to get it into the developer preview. it should also be comparatively low risk, since the main effect is that a function got an additional parameter.

Comment on attachment 180430 [details] [diff] [review] patch v2 a=asa

thanks, checked in