995350 - [SMS] - Display Save button for attached image file name having hidden directories '.gallery/'

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[Punam Dahiya [:pdahiya]]

With fix of bug 992426, Gallery app open activity handler will hide save button for file names having hidden directories such as '.gallery/'

Opening the bug for a fix in SMS app to send filename stripped off of any hidden directories so that opened images can be saved.

See Comments
https://bugzilla.mozilla.org/show_bug.cgi?id=992426#c23

Comment 1

[Punam Dahiya [:pdahiya]]

Julien, can you please help landing this improvement in SMS app over the existing code of 992426 fix because the user will no longer see a choice to save an image for filename with hidden directories.

Comment 2

[Julien Wajsberg [:julienw]]

Needs to be 1.4+ because it's part of the fix to bug 992426.

Comment 3

[Kai-Chih Hu [:khu]]

Joe and Wesley, can we find an owner for this bug? Thanks.

Comment 4

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Give to Steve per offline discussion.

Comment 5

[Julien Wajsberg [:julienw]]

Steve, please reassign back to me if your hands are full, I can take it as well.

Comment 6

[Steve Chung [:steveck]]

(In reply to Julien Wajsberg [:julienw] from comment #5)
> Steve, please reassign back to me if your hands are full, I can take it as
> well.

Sorry I might not have enough time for it this week... :( ,but based on David's suggestion in bug 992426 comment 20, does that mean we should save the attachment into our own folder, or you prefer to just save it in the root?

Comment 7

[Julien Wajsberg [:julienw]]

Our own folder makes sense. I even think that we should make it localizable... like Windows and other OS do.

Comment 8

[Oleg Zasypkin [:azasypkin]]

Hey Julien, what type of storage is supposed to be used? 

I see that we at least have:
1. sdcard
2. pictures\video
3. apps

#1 seems too generic, #2 is OK for media files, but not sure what to do with unknown formats, #3 looks like serving for another purpose and I guess may have very limited size.

Comment 9

[Julien Wajsberg [:julienw]]

Here we simply provide the correct path as filename to the "view" activity, we don't do the save ourselves. Not sure if it answers your question :)

Comment 10

[Julien Wajsberg [:julienw]]

The code to change is https://github.com/mozilla-b2g/gaia/blob/a55eb67d0659327a8e4b64099fc76a634823fecd/apps/sms/js/attachment.js#L236

Comment 11

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] from comment #10)
> The code to change is
> https://github.com/mozilla-b2g/gaia/blob/
> a55eb67d0659327a8e4b64099fc76a634823fecd/apps/sms/js/attachment.js#L236

Yep, I know :) Probably I misunderstood you, but you were talking about own folder, what did you mean? Currently we have filename for cropped image that looks like '/sdcard/.gallery/cropped/5375100743818036.jpeg', how should we modify it before we pass it to 'view' activity?

Comment 12

[Oleg Zasypkin [:azasypkin]]

Forgot to ni? Julien :) Adding now.

Comment 13

[Julien Wajsberg [:julienw]]

I'd first try 'sms-attachments/54657546544.jpeg' (without any prefix) and look whether the Gallery does the right thing :)

Maybe the 'sms-attachments' part could be localized too. Not sure how the Gallery would handle spaces in the directory name either, so maybe we can just file a bug for the possible localization and ask UX.

Comment 14

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] from comment #13)
> I'd first try 'sms-attachments/54657546544.jpeg' (without any prefix) and
> look whether the Gallery does the right thing :)
> 
> Maybe the 'sms-attachments' part could be localized too. Not sure how the
> Gallery would handle spaces in the directory name either, so maybe we can
> just file a bug for the possible localization and ask UX.

Yes, this definitely will work, I'm just wondering that we're polluting root folder with such app-specific folders. If every app decides to create something like this we'll have a mess. So the question is why can't we use something common across the apps like 'appdata/sms/attachments' and 'appdata/any_other_app/....' so that user knows that this is something related to the app (eg. when I see it via FileManager app from MarketPlace) and will not delete it accidentally eg. during sdcard cleanup.

Ideally 'appdata/any_other_app' shouldn't be hardcoded, but rather returned by some common method like 'getAppDataFolderPath' or something similar that will return right path depending on calling app including localized app name.

Just thinking: some of the appdata stuff may be shared across devices with sync. More inspired by http://windows.microsoft.com/en-us/windows-8/what-appdata-folder :)

At the same time it looks like valid case for 'apps' storage type, but I wasn't able to find much info on that.

What do you think about it?

Comment 15

[Julien Wajsberg [:julienw]]

The correct "appdata" could be returned by DeviceStorage, but currently DeviceStorage is used by Gallery, so I don't really know.

Here, I'd like to focus on making the save button work again, but we can file a bug for your proposal which definitely makes sense and request UX feedback :)

Comment 16

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] from comment #15)
> The correct "appdata" could be returned by DeviceStorage, but currently
> DeviceStorage is used by Gallery, so I don't really know.
> 
> Here, I'd like to focus on making the save button work again, but we can
> file a bug for your proposal which definitely makes sense and request UX
> feedback :)

Ok, got it, we just need quick fix for now :) Will do that and file separate bug for 'appdata'.

Comment 17

[Oleg Zasypkin [:azasypkin]]

Attachment: GitHub pull request URL

Hopefully temporal solution to return Save button back :)

Comment 18

[Julien Wajsberg [:julienw]]

Comment on attachment 8413408 [details] [review]
GitHub pull request URL

r=me, thanks !
You know what to do :) (commit log, checkin-needed)

Comment 19

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] from comment #18)
> Comment on attachment 8413408 [details] [review]
> GitHub pull request URL
> 
> r=me, thanks !
> You know what to do :) (commit log, checkin-needed)

Thanks! Done.

Comment 20

[Carsten Book [:Tomcat]]

master: https://github.com/mozilla-b2g/gaia/commit/17c319282315f5a86b2c31a84fe3e4c68d568fd8

Comment 21

[Ryan VanderMeulen [:RyanVM]]

v1.4: https://github.com/mozilla-b2g/gaia/commit/82915bc4bb3862357efb45c917bcf5554b306479