1. Title: Unable to save the received cropped image 2. Precondition: Should be able to send and receive the MMS messages 3. Tester's Action: New - Message - Gallery - select image - Crop - send message 4. Detailed Symptom (ENG.) : The received cropped image cannot be saved 5. Expected: Should be able to save the cropped image 6. Reproducibility: Y 1) Frequency Rate : 100% 7. Gaia Master/v1-train: Reproduced on v1-train 8. Gaia Revision: 613e69ee0a009399130ad2cbb8b9d0462cd1fc70 9. Personal email id: email@example.com
blocking-b2g: --- → leo+
Priority: -- → P1
Target Milestone: --- → 1.1 QE4 (15jul)
Taking a look!
Assignee: nobody → fbsc
Whiteboard: [TD-58436] → [TD-58436], [u=commsapps-user c=messaging p=0]
Created attachment 772002 [details] Pointer to Pull Request: https://github.com/mozilla-b2g/gaia/pull/10833 Please review the attached patch.
Comment on attachment 772002 [details] Pointer to Pull Request: https://github.com/mozilla-b2g/gaia/pull/10833 Leo, I don't have the authority to review a bug. You have to request the review to the module owner, like David Flanagan or Dale Harvey. Since David is PTO now, I redirect the review to Dale. Dale, Sorry, David is PTO now. I think the best reviewer is you.
Attachment #772002 - Flags: review?(johu) → review?(dale)
Comment on attachment 772002 [details] Pointer to Pull Request: https://github.com/mozilla-b2g/gaia/pull/10833 update mime type to html
Im gonna move this to Gallery and re-assigning this properly.
Component: Gaia::SMS → Gaia::Gallery
Whiteboard: [TD-58436], [u=commsapps-user c=messaging p=0] → [TD-58436], [u=commsapps-user c=messaging p=1]
Comment on attachment 772002 [details] Pointer to Pull Request: https://github.com/mozilla-b2g/gaia/pull/10833 David, I've seen this fix is related with Gallery, could you take a look? Thanks! Gracias :)
Summary: [MMS] Unable to save the received cropped image → [Gallery][MMS] Unable to save the received cropped image
Comment on attachment 772002 [details] Pointer to Pull Request: https://github.com/mozilla-b2g/gaia/pull/10833 Thanks for the patch, since David is on PTO I cleared the review for him. Cleared the review for me, The code here is correct, but it the commit needs fixing before it can be merged, both files have an unncessary mode change (644->755), there is an indentation nit, and the commit message doesnt follow our guidelines (https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Platform/Gaia/Hacking#Submitting_a_patch has the details) Add me as r? on the ammended commit, thanks
Review changes are squashed.Please reiew the attached patch.
Comment on attachment 772002 [details] Pointer to Pull Request: https://github.com/mozilla-b2g/gaia/pull/10833 Add feedback to Dominic. We had discussed this bug today. He had some concerns on this patch.
Whiteboard: [TD-58436], [u=commsapps-user c=messaging p=1] → [TD-58436], [u=commsapps-user c=messaging p=1], TaipeiMMS
To save in the open activity of gallery, there should be two options called "allowSave" and "filename". I think if one app(like sms) is going to open an image with save option, then "allowSave" should be set to true, and "filename" should be set to a certain name with extension. Although it will be great that gallery can handle the condition when the filename is missing an extension, but since sms already knows allowSave is set to true, it should also try to avoid an uncertain filename that probably cause a save failure.
Comment on attachment 772002 [details] Pointer to Pull Request: https://github.com/mozilla-b2g/gaia/pull/10833 I wasnt able to diagnose which error is causing the mms to fail sending, however this is patching the wrong file, when an mms attaches an image it is sending a pick activity to the gallery which doesnt load open.html Sending still failed for me with this patch applied, its possible for a different reason but either way the STR need clarified for me to review, as far as I know there is no path to save an image when composing an mms
Attachment #772002 - Flags: review?(dale) → review-
The gallery should probably auto-fix extensions / normalize / de-duplicate filenames on the saving option? What happens if the file name is already taken in gallery? What happens with attachments sent from iPhone (as they don't have any filenames associated usually and are just called something like "200") "allowSave" should just turn on the save button, and the filename should be fixed by gallery IMO
In reply to Dale Harvey , STR: 1. This issue is not related to sending a mms file(image attachment). 2. This is basically , the user who receives a MMS( cropped image, sender has cropped the image ) is not able to save the image file. The patch uploaded only fixes "saving of a received cropped image file" . In reply to Corey Frang , First we are checking in the filename for presence of extension, if extension is not present we add the extension based on mimetype , refer the patch . After that if filename is already taken is gallery , it uses getUnusedFilename() function (refer ...shared\js\device_storage_utils.js) to create a new filename as below, ..... var newname = dir + base + '_' + (version + 1) + ext; ..... Here it doesn't matter whether its a character based filename or numeric (the function getUnusedFilename() creates new file name appended with a version). Hi Dominic, I have a doubt here, so we should be adding the "allowsave" and " filename" as part of gallery open activity ? Iam not clear about "allowsave" and "filename". Can you please elaborate a bit more. Thanks,
I think actually there are two issues here: 1. In the open activity of gallery, when we try to save an image attachment without an extension, it might be some cropped image or raw data image, and since DeviceStorage.addNamed() does not allow to save a file without an extension, we cannot save it directly unless we add one extension for it. 2. In sms, if the sent/received attachment has no filename, it will just give a very simple name called "untitled" without any description/hint or extension. And if we fixed issue 1 with some method like MimeMapper, the users might see the saved files with filenames like "untitled_1.jpg", "untitled_2.amr",... which users are unable to recognize where or which app they used to save these files, also there is no specific folder for saving these attachments, so when users are trying to copy the saved attachments from sms by PC, they don't know where and which files they should look for. In fact, 3GPP TS 26.140 - "Multimedia Messaging Service (MMS); Media formats and codecs" has defined the minimum set of supported formats for MMS: http://www.3gpp.org/ftp/Specs/html-info/26140.htm Base on the above spec, it's possible that we can use the mimetype to decide the extension because we should know the supported formats for MMS, and if it's a raw media data that has no extension, we can fix it then issue 1 won't be a problem. Also, to have better UX, it will be nice that sms use some meaningful filenames or add prefix to save the attachment in a specific folder, instead of using the "untitled" name for every media attachments. So I would like to propose to fix issue 2 more then fix issue 1 because it can prevent more potential bugs in the other scenarios, or fix the two issues together will be better.
(In reply to Corey Frang [:gnarf] [:gnarf37] from comment #13) > What happens if the file name is already taken in gallery? Gallery used shared/js/device_storage_utils.js to handle this but it assumes the filename contains extension. > What happens with attachments sent from iPhone (as they don't have any > filenames associated usually and are just called something like "200") If the mimetype is not something like "application/octet-stream", it should be able to match one open activity to view the attachment. I remember you mentioned it's not common in bug 884722 comment 8? (In reply to Leo from comment #14) > Hi Dominic, > I have a doubt here, so we should be adding the "allowsave" and " filename" > as part of gallery open activity ? > Iam not clear about "allowsave" and "filename". Can you please elaborate a > bit more. The "allowsave" and "filename" are the options for the open activity of gallery and they are already used, you can see how sms invoke MozActivity in attachment.js.
Created attachment 779658 [details] [diff] [review] gallery_crop.patch Hi Dominic, I have a created patch, this patch fixes the issue. Please review it , and add comments here. Thanks,
Comment on attachment 779658 [details] [diff] [review] gallery_crop.patch Leo, I think your patch only fix the issue when gallery is trying to save a file without extension, but for music and video, when saving audio/video like that, the same issue whill occur, so we better fix them together. I guess we can modify shared/js/device_storage_utils.js to fix the missing extension, instead of changing gallery/music/video apps, more, other apps that use device_storage_utils.js can also avoid this issue.
Created attachment 780885 [details] [diff] [review] galleryext.patch Hi Dkuo, I have uploaded a patch as per your suggestions, please review it and let me know your comments . Thanks,
Comment on attachment 780885 [details] [diff] [review] galleryext.patch Leo, I think there are two major issue in this patch. First, I think the pickedFile is from deviceStorage, so it should have an extension or deviceStorage won't return it to gallery, and actually we don't need MimeMapper to guess the extension, you can just use the original extension from the pickedFile. Second, I think this patch only fixes this issue in gallery, I am afraid there might be some audio or video will have the some problem, so I would like to see some patch implements the way I mentioned in comment 18.
Since I am the person who proposed the fix in comment 15, so I am going to take this bug and implement the fix.
Assignee: leo.bugzilla.gaia → dkuo
moving to leo? not been looked at 7/30. Please nominate if looking at bug.
blocking-b2g: leo+ → leo?
Update: I am in the Media team meet-up week so don't have a chance to work on it, any one feel free to steal it if you want to.
Hi, Is this a corner case or is this a general case? Please confirm.
Triage - partner will check if this is still blocking and feedback blocking decision on their end. Preeti, this is a general case that can be systematically reproduced with the STR provided. (In reply to Preeti Raghunath(:Preeti) from comment #24) > Hi, > > Is this a corner case or is this a general case? > > Please confirm.
Hi Dominic, Are you looking at this bug? Mike, Do you have an engineer looking at this bug from your team?
David, Please Preeti's comment #26 re: assignee. Preeti, David runs the Comms Apps team that handles MMS.
Flags: needinfo?(mlee) → needinfo?(dscravaglieri)
Currently I am not working on this because it had became leo? then leo+ so I have pulled out my resources for now, feel free to take it if anyone would like to work on it, or I can come back to this one after I finish my current work.
Re-assign to nobody first because I am not working on this.
Assignee: dkuo → nobody
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 895726
I found this bug is a duplicate of bug 895726. These two bugs are related to the correct behavior of blob saving of gallery app.
You need to log in before you can comment on or make changes to this bug.