Closed Bug 1072978 Opened 5 years ago Closed 5 years ago

'Set image as' option does not work when /mnt/sdcard/DCIM is missing

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 36
Tracking Status
firefox33 --- affected
firefox34 --- affected
firefox35 --- affected
fennec + ---

People

(Reporter: cos_flaviu, Assigned: domivinc)

References

Details

Attachments

(1 file, 1 obsolete file)

Environment: 
Device: Google Nexus 7 (Android 4.4.4);
Build: Nightly 35.0a1 (2014-09-25);

Steps to reproduce:
1. Load a website which contains images (e.g. mozilla.org);
2. Long tap on a image;
3. Select image from the context menu;
4. Tap on 'Set image as'.

Expected result:
A list with the available options is displayed.

Actual result:
'Unable to save image' toast notification is displayed.

Notes:
Not reproducible on Nexus 4 (Android 4.4.4).

The following exception is thrown in logcat:
Failed to insert image
java.io.FileNotFoundException: No such file or directory
at android.database.DatabaseUtils.readExceptionWithFileNotFoundExceptionFromParcel(DatabaseUtils.java:146)
at android.content.ContentProviderProxy.openAssetFile(ContentProviderNative.java:611)
at android.content.ContentResolver.openAssetFileDescriptor(ContentResolver.java:922)
at android.content.ContentResolver.openOutputStream(ContentResolver.java:669)
at android.content.ContentResolver.openOutputStream(ContentResolver.java:645)
at android.provider.MediaStore$Images$Media.insertImage(MediaStore.java:902)
at org.mozilla.gecko.GeckoApp.setImageAs(GeckoApp.java:967)
at org.mozilla.gecko.GeckoApp.handleMessage(GeckoApp.java:595)
at org.mozilla.gecko.BrowserApp.handleMessage(BrowserApp.java:1482)
at org.mozilla.gecko.EventDispatcher.dispatchEvent(EventDispatcher.java:168)
at org.mozilla.gecko.GeckoAppShell.handleGeckoMessage(GeckoAppShell.java:2327)
at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method)
at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method)
at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method)
at org.mozilla.gecko.GeckoAppShell.runGecko(GeckoAppShell.java:365)
at org.mozilla.gecko.GeckoThread.run(GeckoThread.java:186)
Maybe not, works on my Nexus 5.
Oh from that report, "DCIM directory havn't created in sdcard"

Is that some stupid hard requirement? Flaviu, can you check your Nexus 7?
tracking-fennec: --- → ?
Confirmed. Typical Android.

Renamed my /mnt/sdcard/DCIM to DCIMM and reproduced this bug.
Keywords: reproducible
Summary: 'Set image as' option does not work on Nexus 7 → 'Set image as' option does not when /mnt/sdcard/DCIM is missing
Summary: 'Set image as' option does not when /mnt/sdcard/DCIM is missing → 'Set image as' option does not work when /mnt/sdcard/DCIM is missing
Assignee: nobody → liuche
tracking-fennec: ? → +
Dominique - Let us know if you want to take a shot at this one too.
Blocks: 1062904
Flags: needinfo?(domivinc)
Is the solution to make a DCIM folder if it doesn't exist?
Ahh, I didn't realize. The method is documented that it might throw, and we're just catching and logging the exception.

Making the "fake" folder doesn't sound crazy to me (long as we have a comment explaining why we do it).
Hi Mark, yes I can fix this one. 
I will test the DCIM path before the call to the method "Media.insertImage". If the path doesn't exist, I will try to create it. If the creation fails, I will display a toast message: "Unable to create image folder".
Flags: needinfo?(domivinc)
Attached patch patch1072978.diff (obsolete) — Splinter Review
Attachment #8496066 - Flags: review?(mark.finkle)
Comment on attachment 8496066 [details] [diff] [review]
patch1072978.diff

>diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java

>             if (image != null) {

Let's add a comment about why this check exists. Something like:

// Some devices don't have a DCIM folder and the Media.insertImage call will fail.

>+                File dcimDir = Environment.getExternalStoragePublicDirectory(
>+                        Environment.DIRECTORY_PICTURES);

No wrapping needed here. Just keep it on one line.

>+                if (dcimDir.mkdirs() == false && dcimDir.isDirectory() == false) {

We can shorten this I think:
                  if (!dcimDir.mkdirs() && !dcimDir.isDirectory()) {


>+                    Toast.makeText((Context) this, R.string.create_image_folder_fail, Toast.LENGTH_SHORT).show();

Let's just use the same general string:  R.string.set_image_path_fail
Attachment #8496066 - Flags: review?(mark.finkle) → feedback+
Code review changes are implemented in this new patch.
Attachment #8496066 - Attachment is obsolete: true
Attachment #8496394 - Flags: review?(mark.finkle)
Attachment #8496394 - Flags: review?(mark.finkle) → review+
Status: NEW → ASSIGNED
A try push is required for this patch. I can't do it, I'm still waiting on a voucher to get commit access on try server (Bug 1074978).
Try run in comment 13 looks green to me
https://hg.mozilla.org/mozilla-central/rev/aee7f5790478
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Target Milestone: Firefox 35 → Firefox 36
The bug is still reproducible in build 36.0a1 (2014-10-14);
Tested on Nexus 7 (Android 4.4.4).
(In reply to Flaviu Cos, QA [:flaviu] from comment #17)
> The bug is still reproducible in build 36.0a1 (2014-10-14);
> Tested on Nexus 7 (Android 4.4.4).

Can you post the call stack again?
(In reply to Mark Finkle (:mfinkle) from comment #18)
> Can you post the call stack again?

Failed to insert image
java.io.FileNotFoundException: No such file or directory
at android.database.DatabaseUtils.readExceptionWithFileNotFoundExceptionFromParcel(DatabaseUtils.java:146)
at android.content.ContentProviderProxy.openAssetFile(ContentProviderNative.java:611)
at android.content.ContentResolver.openAssetFileDescriptor(ContentResolver.java:922)
at android.content.ContentResolver.openOutputStream(ContentResolver.java:669)
at android.content.ContentResolver.openOutputStream(ContentResolver.java:645)
at android.provider.MediaStore$Images$Media.insertImage(MediaStore.java:902)
at org.mozilla.gecko.GeckoApp.setImageAs(GeckoApp.java:988)
at org.mozilla.gecko.GeckoApp.handleMessage(GeckoApp.java:608)
at org.mozilla.gecko.BrowserApp.handleMessage(BrowserApp.java:1567)
at org.mozilla.gecko.EventDispatcher.dispatchEvent(EventDispatcher.java:168)
at org.mozilla.gecko.GeckoAppShell.handleGeckoMessage(GeckoAppShell.java:2348)
at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method)
at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method)
at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method)
at org.mozilla.gecko.GeckoAppShell.runGecko(GeckoAppShell.java:369)
at org.mozilla.gecko.GeckoThread.run(GeckoThread.java:190)
Without a DCIM card Firefox won't be able to save the image. You probably won't be able to take a picture with your device.
The fix creates the folder if the folder is missing in the DCIM card. If the folder cannot be created (no DCIM for instance) you should have a toast message. Did you get a toast message?
(In reply to Flaviu Cos, QA [:flaviu] from comment #17)
> The bug is still reproducible in build 36.0a1 (2014-10-14);
> Tested on Nexus 7 (Android 4.4.4).

This is working on my Nexus 7. Maybe you have it connected via ADB and that is causing a problem?
(In reply to Mark Finkle (:mfinkle) from comment #21)
> This is working on my Nexus 7. Maybe you have it connected via ADB and that
> is causing a problem?
It is reproducible even if it is not connected to the PC.
I tried also to disable the 'Developer Tools' but still doesn't work.
'Unable to save image' toast is displayed. 

(In reply to Dominique Vincent [:domivinc] from comment #20)
> Without a DCIM card Firefox won't be able to save the image. 
I can successfully save the image via 'Save Image' option from the context menu.
(In reply to Flaviu Cos, QA [:flaviu] from comment #22)

> (In reply to Dominique Vincent [:domivinc] from comment #20)
> > Without a DCIM card Firefox won't be able to save the image. 
> I can successfully save the image via 'Save Image' option from the context
> menu.
In this case, what is the path to the downloaded image ?
(In reply to Dominique Vincent [:domivinc] from comment #23)
> In this case, what is the path to the downloaded image ?

The path is /sdcard/Download/
After taking a picture with the camera of the device, can you access to the picture and get the path of the picture?
The path (Environment.DIRECTORY_PICTURES) used by Firefox should be the same.
You can also try to reboot your device.
(In reply to Dominique Vincent [:domivinc] from comment #25)
> After taking a picture with the camera of the device, can you access to the
> picture and get the path of the picture?
> The path (Environment.DIRECTORY_PICTURES) used by Firefox should be the same.
> You can also try to reboot your device.

I found the issue :)
Nexus 7 2012 doesn't have a back camera, so it doesn't come with build in camera app.
After I installed a camera app from the play store the DCIM folder is created.
After that the 'Set image as' option works as expected.

Pictures taken with the camera are stored in: /sdcard/DCIM/Camera/
Does that mean we need to re-adjust the logic here? New bug?
You need to log in before you can comment on or make changes to this bug.