Closed Bug 572441 Opened 9 years ago Closed 9 years ago

HelperApp not opened correctly

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Maemo
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wolfiR, Unassigned)

References

Details

Attachments

(1 file)

While implementing a new feature I found this:
I _think_ this might affect MAEMO5 builds of Fennec as well (not tested but checking the code I'm wondering why it would work there).

Trying to open a file (instead of just saving it) I'm ending up here:
http://mxr.mozilla.org/mobile-browser/source/components/HelperAppDialog.js#97
aLauncher.launchWithApplication(null, false);

Following this codepath basically runs through the following methods:
nsExternalAppHandler::LaunchWithApplication
nsExternalAppHandler::CreateProgressListener
nsExternalAppHandler::SetWebProgressListener
nsExternalAppHandler::ExecuteDesiredAction

http://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/nsExternalHelperAppService.cpp#1894

There we check for the preferredAction which defaults to saveToDisk.
All along the way outlined above I cannot see that the preferred action is actually set to something and so I'm ending up in a download-only situation instead of going further with OpenWithApplication().

Given the fact that we check if (aLauncher.MIMEInfo.hasDefaultHandler) and call launchWithApplication just in that case I'd think the defining the preferred default action would be a possible fix.
I'll attach a patch.
Attached patch patchSplinter Review
Attachment #451623 - Flags: review?(mark.finkle)
Component: Linux/Maemo → General
OS: Linux → Linux (embedded)
Hardware: All → ARM
Comment on attachment 451623 [details] [diff] [review]
patch

>diff --git a/components/HelperAppDialog.js b/components/HelperAppDialog.js

>       if (choice == 0)
>         aLauncher.saveToDisk(null, false);
>-      else if (choice == 1)
>+      else if (choice == 1) {
>+        aLauncher.MIMEInfo.preferredAction =
>+          Components.interfaces.nsIMIMEInfo.useSystemDefault;

Maybe we should only set aLauncher.MIMEInfo.preferredAction if it is not already set to something sane

This looks fine, but I'd like to test it before r+ and we need downloads to work for that to happen
Blocks: 584225
Attachment #451623 - Flags: review?(mark.finkle) → review?(blassey.bugs)
This used to work as is, right? Do we know when that broke?
assuming that this is a regression, it would be helpful to know when it broke
(In reply to comment #4)
> assuming that this is a regression, it would be helpful to know when it broke

I will try to find out.
Jan is now in vacation, i wasnt able to find the point where it was broken.
Comment on attachment 451623 [details] [diff] [review]
patch

There's nothing wrong with this patch, but I am concerned that something has broken and we're just covering it up. Please file a follow up bug to figure out when it broke and why.
Attachment #451623 - Flags: review?(blassey.bugs) → review+
pushed:
http://hg.mozilla.org/mobile-browser/rev/d89f5aa815bc
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
My guess back then was that bug 570156 had something to do with it but I have no proof and I haven't checked when I ran into it since my development platform haven't had open handlers at that time.
You need to log in before you can comment on or make changes to this bug.