289842 - Make nsIHelperAppLauncher inherit from nsICancelable

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect, P1)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

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 1

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 2

[Darin Fisher]

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

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

> 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

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v2

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

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 6

[Asa Dotzler [:asa]]

Comment on attachment 180430 [details] [diff] [review]
patch v2

a=asa

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

thanks, checked in