Closed Bug 183848 Opened 22 years ago Closed 20 years ago

autoDispatch isn't working for some downloads (need to use Launch Services directly)

Categories

(SeaMonkey :: General, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.7alpha

People

(Reporter: sdagley, Assigned: mikepinkerton)

References

Details

(Keywords: topembed+, Whiteboard: edt_c3,edt_d3)

Attachments

(2 files, 1 obsolete file)

nsOSHelperAppService::LaunchAppWithTempFile is being called to dispatch the file
downloaded but the aMIMEInfo->GetPreferredApplicationHandler call on the
nsIMIMEInfo passed in doesn't retrieve an application.  This only happens for
some file types which leads me to believe we're running into a problem between
LaunchServices and InternetConfig again.  My proposed fix is to call
LSGetApplicationForItem() for the tempfile and see if launchServices does in
fact find a handler.
Attached patch Patch as described (obsolete) — Splinter Review
Fixed
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Target Milestone: --- → Chimera0.7
Excellent.. This now launches the default app for .pdf files and .rm files after
dl is completed. Tested with different default apps for .pdf (Acrobat and
Preview.app) and RealOne player (default app for .rm files). Added
user_pref("browser.download.autoDispatch", true); to my user.js. Tested with the
2002-12-06-04 NB under 10.2.2.
Status: RESOLVED → VERIFIED
Reopening as this needs to be landed on the Mozilla trunk.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Target Milestone: Camino0.7 → Camino0.8
Comment on attachment 108446 [details] [diff] [review]
Patch as described

Adding r/sr requests for trunk landing
Attachment #108446 - Flags: superreview?(sfraser)
Attachment #108446 - Flags: review?(ccarlen)
Comment on attachment 108446 [details] [diff] [review]
Patch as described

r=ccarlen
Attachment #108446 - Flags: review?(ccarlen) → review+
I have an alternative fix in bug 197745 that doesn't require #ifdefs in that
code (which is to do autoexanding from the frontend). I think it's cleaner.
Changing Product to Browser as this isn't a Camino speciifc change (and as Simon
commented he has a different fix for Camino that doesn't use 
nsOSHelperAppService::LaunchAppWithTempFile)
Component: Downloading → Browser-General
Product: Camino → Browser
Target Milestone: Camino0.8 → mozilla1.4alpha
Version: unspecified → Trunk
Clarify what the patch is about. Yes, we need this.
Summary: autoDispatch isn't working for some downloads → autoDispatch isn't working for some downloads (need to use Launch Services directly)
->sfraser
Assignee: sdagley → sfraser
Status: REOPENED → NEW
Keywords: topembed
Simon, since bug 197745 is fixed, should this be duped to it and closed?
Comment on attachment 108446 [details] [diff] [review]
Patch as described

No, we still need this patch.
Attachment #108446 - Flags: superreview?(sfraser) → superreview+
Discussed in edt bug triage.  Plussing.
Keywords: topembedtopembed+
Whiteboard: edt_c3,edt_d3
Checked in.
Status: NEW → RESOLVED
Closed: 22 years ago21 years ago
Resolution: --- → FIXED
this is interesting. this patch was merged to the trunk (at time of checkin)
rather wrongly.

This patch, as attached, has basically this (correct) logic:
if (application) {
 ...use it...
} else {
 ... ask launch services ...
}

As checked in, it does this, entirely different, thing:
if (aMimeInfo) {
 ... use the mimeinfo's application, return if none available ...
} else {
  ...this code is basically unreachable...
  ...were it reachable, it would talk to launchservices...
}


Presumably the following checkin is sorta responsible for this:
bug 86640, which changed the |if (application) { ... }| to |if (!application)
return some_error;|

my point is:
this code currently does NOTHING AT ALL. reopening bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Someone who has a mac build and can test needs to fix this up....
taking
Assignee: sfraser → sdagley
Status: REOPENED → NEW
note, this code will move to the new nsMIMEInfoMac.cpp
(uriloader/exthandler/mac) soon (i.e. hopefully before 1.7alpha)

(will happen in bug 78919)
i have a new patch (which i can't diff because cvs is down) but i have no idea
how to test it. the instructions here aren't very informative.

can someone provide a testcase?
Assignee: sdagley → pinkerton
Target Milestone: mozilla1.4alpha → mozilla1.7alpha
updated patch with correct merge
Attachment #108446 - Attachment is obsolete: true
Attachment #141655 - Flags: review?(cbiesinger)
Attachment #141655 - Flags: superreview?(sfraser)
+      FSRef tempFileRef;
+      tempFile->GetFSRef(&tempFileRef);

Does the temp file have the right extension (and file type?) at this point?
Also, a diff -w might be clearer.
Attached patch diff -w of aboveSplinter Review
diff -w per smfr's request
Attachment #141655 - Flags: superreview?(sfraser) → superreview+
Attachment #141655 - Flags: review?(cbiesinger) → review+
please test this use case though:
o) set some application as the helper application for a file type
o) remove/rename that application
o) load a file with that mime type

something sensible should happen :) I suspect this may cause this new code to be
invoked, but I am not sure... especially I am not sure if it is supposed to be
invoked...
The oriinal problem was apparently the InternetConfig database of helper apps
would get out of sync with the LaunchServices database.  We'd query IC which
would deny there was a handler for the file type after download but the user
could double-click on the file and LaunchServices would dispatch it.  How you
force that to happen I don't know so I'd test by commenting out the if
(application) block and force the code path thru the LS dispatching code.
landed
Status: NEW → RESOLVED
Closed: 21 years ago20 years ago
Resolution: --- → FIXED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: