Closed
Bug 1072978
Opened 10 years ago
Closed 10 years ago
'Set image as' option does not work when /mnt/sdcard/DCIM is missing
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox33 affected, firefox34 affected, firefox35 affected, fennec+)
RESOLVED
FIXED
Firefox 36
People
(Reporter: cos_flaviu, Assigned: domivinc)
References
Details
Attachments
(1 file, 1 obsolete file)
2.31 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
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)
Reporter | ||
Updated•10 years ago
|
Comment 1•10 years ago
|
||
4.4.4 issue? https://code.google.com/p/android/issues/detail?id=75447
Comment 2•10 years ago
|
||
Maybe not, works on my Nexus 5.
Comment 3•10 years ago
|
||
Oh from that report, "DCIM directory havn't created in sdcard" Is that some stupid hard requirement? Flaviu, can you check your Nexus 7?
Updated•10 years ago
|
tracking-fennec: --- → ?
Comment 4•10 years ago
|
||
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
Updated•10 years ago
|
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
Updated•10 years ago
|
Assignee: nobody → liuche
tracking-fennec: ? → +
Comment 5•10 years ago
|
||
Dominique - Let us know if you want to take a shot at this one too.
Blocks: 1062904
Flags: needinfo?(domivinc)
Comment 6•10 years ago
|
||
Is the solution to make a DCIM folder if it doesn't exist?
Comment 7•10 years ago
|
||
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).
Assignee | ||
Comment 8•10 years ago
|
||
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".
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(domivinc)
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8496066 -
Flags: review?(mark.finkle)
Comment 10•10 years ago
|
||
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+
Assignee | ||
Comment 11•10 years ago
|
||
Code review changes are implemented in this new patch.
Attachment #8496066 -
Attachment is obsolete: true
Attachment #8496394 -
Flags: review?(mark.finkle)
Updated•10 years ago
|
Attachment #8496394 -
Flags: review?(mark.finkle) → review+
Updated•10 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 12•10 years ago
|
||
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).
Comment 13•10 years ago
|
||
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=3af0030e469e
Assignee: liuche → domivinc
Assignee | ||
Comment 14•10 years ago
|
||
Try run in comment 13 looks green to me
Assignee | ||
Updated•10 years ago
|
Keywords: reproducible → checkin-needed
Comment 15•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/aee7f5790478
Keywords: checkin-needed
Comment 16•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/aee7f5790478
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Updated•10 years ago
|
Target Milestone: Firefox 35 → Firefox 36
Reporter | ||
Comment 17•10 years ago
|
||
The bug is still reproducible in build 36.0a1 (2014-10-14); Tested on Nexus 7 (Android 4.4.4).
Comment 18•10 years ago
|
||
(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?
Reporter | ||
Comment 19•10 years ago
|
||
(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)
Assignee | ||
Comment 20•10 years ago
|
||
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?
Comment 21•10 years ago
|
||
(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?
Reporter | ||
Comment 22•10 years ago
|
||
(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.
Assignee | ||
Comment 23•10 years ago
|
||
(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 ?
Reporter | ||
Comment 24•10 years ago
|
||
(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/
Assignee | ||
Comment 25•10 years ago
|
||
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.
Reporter | ||
Comment 26•10 years ago
|
||
(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/
Comment 27•10 years ago
|
||
Does that mean we need to re-adjust the logic here? New bug?
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•