Closed Bug 613590 Opened 14 years ago Closed 3 years ago

No alert shown if we try to download with no sd-card attached

Categories

(Firefox for Android Graveyard :: General, defect, P5)

All
Android
defect

Tracking

(fennec+)

RESOLVED INCOMPLETE
Tracking Status
fennec + ---

People

(Reporter: wesj, Unassigned)

References

Details

(Keywords: uiwanted)

Attachments

(1 file)

We don't provide any feedback about why we cancel a download if there is no SD-Card attached to the system. In a broader sense, I'm not sure we are showing alerts for any download failures. There's two problems here I think:

1.) We are returning a NS_ERROR_FAILURE when there is no SD-Card attached. We should return an appropriate error code so that an appropriate message can be pulled from our strings (NS_ERROR_FILE_ACCESS_DENIED?)
http://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/nsExternalHelperAppService.cpp#483

2.) When we do get to the prompter section of code, we can't get one and fail to show the prompt. Not sure why we can't get the prompter:
http://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/nsExternalHelperAppService.cpp#1906

STR:
0.) Mount your phone to a machine so that the sd card is mounted as a USB mass storage device
1.) Long tap on an image
2.) Choose "Save Image"

Result: Nothing
Expected: An error pops up explaining why we cancelled the download
As I noted in bug 610383, getTargetFile fails when I try to save an image on my phone with no SD card attached.
tracking-fennec: --- → ?
Flags: in-litmus?
tracking-fennec: ? → 2.0-
Attached patch Patch v1Splinter Review
Switches this to use the nsIPromptService instead, which should be ok on mobile whether we pass it a window or not. Also changes the error to NS_ERROR_FILE_ACCESS_DENIED.
Attachment #494061 - Flags: review?(dwitte)
I thought the prompter issue was fixed recently?
I'm still not seeing an alert with the nightly. As far as I know this isn't fixed.
(In reply to comment #5)
> I'm still not seeing an alert with the nightly. As far as I know this isn't
> fixed.

OK, thanks for checking.
Comment on attachment 494061 [details] [diff] [review]
Patch v1

Kicking to bz, but this explicitly isn't a blocker, so you should probably explain why it should be. :)
Attachment #494061 - Flags: review?(dwitte) → review?(bzbarsky)
Just using the prompt service seems wrong to me.  Who is this "we" who has no prompt hanging off the window context, and why are you in that situation?
Comment on attachment 494061 [details] [diff] [review]
Patch v1

r- pending response to comment 8.
Attachment #494061 - Flags: review?(bzbarsky) → review-
Sorry. Haven't had time to dig into this, and it is not blocking.

It seems likely to me that what is happening is that the original calls to the nsExternalAppHelperService come from the child process. When the are passed up to the parentProcess they aren't given an mWindowContext anymore?

That happens with things like the prompt service in Fennec. But I haven't looked into it deeply to confirm that is the problem here. Just my gut guess.
OK, that seems fine.  I don't have a problem with falling back to the prompt service if we fail to get a prompt off the context.  Would that still fix the issue here?
That said, maybe we should be passing in a context that can hand out prompts...
Whiteboard: [fennec-4.1?]
tracking-fennec: - → 7+
Whiteboard: [fennec-4.1?]
Assignee: nobody → wjohnston
tracking-fennec: 7+ → ?
tracking-fennec: ? → 8+
Wes, are we waiting for UX here? What's blocking it?
tracking-fennec: 8+ → +
Assignee: wjohnston → nobody
Product: Fennec → Fennec Native
Whiteboard: [mentor=wesj]
Mentor: wjohnston
Whiteboard: [mentor=wesj]
filter on [mass-p5]
Priority: -- → P5
Hey!
I would love to work on this bug. Will this be appropriate for me or should I look for some other bugs?
(In reply to Prabhjyot Sodhi [:psd] from comment #16)
> Hey!
> I would love to work on this bug. Will this be appropriate for me or should
> I look for some other bugs?

This bug hasn't seen any action in a while, and I'm not sure if it's still relevant.

wesj, is this still something we want to do?
Flags: needinfo?(wjohnston)
This probably works in Fennec now (since we aren't multi-process anymore). But I bet the bug still exists in the platform. I'm not sure who would be a good person to mentor you on it. Maybe someone like ehsan does?
Flags: needinfo?(wjohnston) → needinfo?(ehsan)
Sorry, I will be on vacation for three weeks from the middle of next week, won't have enough time before then.
Flags: needinfo?(ehsan)
Mentor: wjohnston2000
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INCOMPLETE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: