Closed
Bug 289842
Opened 19 years ago
Closed 19 years ago
Make nsIHelperAppLauncher inherit from nsICancelable
Categories
(Core Graveyard :: File Handling, defect, P1)
Core Graveyard
File Handling
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.8beta2
People
(Reporter: Biesinger, Assigned: Biesinger)
References
Details
(Keywords: arch)
Attachments
(1 file, 1 obsolete file)
17.14 KB,
patch
|
asa
:
approval1.8b2+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•19 years ago
|
||
Attachment #180333 -
Flags: superreview?(darin)
Attachment #180333 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta2
Updated•19 years ago
|
Attachment #180333 -
Flags: review?(bzbarsky) → review+
Comment 2•19 years ago
|
||
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
Attachment #180333 -
Flags: superreview?(darin) → superreview+
Assignee | ||
Comment 3•19 years ago
|
||
> 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
Assignee | ||
Comment 4•19 years ago
|
||
Attachment #180333 -
Attachment is obsolete: true
Attachment #180430 -
Flags: approval1.8b2?
Assignee | ||
Comment 5•19 years ago
|
||
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•19 years ago
|
||
Comment on attachment 180430 [details] [diff] [review] patch v2 a=asa
Attachment #180430 -
Flags: approval1.8b2? → approval1.8b2+
Assignee | ||
Comment 7•19 years ago
|
||
thanks, checked in
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•