838042 - [Browser][User Story] Image save from webpage

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Browser, defect, P1)

Description

[Peter Dolanjski [:pdol]]

UCID: Browser-112

User Story:
As a user, I want the ability to save an image from the webpage (currently supported MIME types) so that I can later access that image from my Gallery application.

Comment 1

[Ben Francis [:benfrancis]]

Justin, can you comment on how we might implement this feature? Presumably we would need to extend the Browser API to make this possible?

Comment 2

[Justin Lebar (not reading bugmail)]

I think we'd just have to add a bit of information to the contextmenu event.

Comment 3

[[:fabrice] Fabrice Desré]

We already set the image url on the contextmenu event from there:
https://mxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementChild.js#497

Comment 4

[[:fabrice] Fabrice Desré]

Attachment: patch

That works fine in my tests. I did not add any UI feedback notifying the user about success or failure, because a desktop notification seems a bit too heavy, and I'm not sure what else we can use.

Comment 5

[Rob MacDonald [:robmac]]

Hopefully this flow doesn't come too late, but I've posted a draft to dropbox:

https://www.dropbox.com/s/bdgx8ofucuzc3vj/browser-saveimage-130225-DRAFT.pdf

This is subject to review by the browser UI folks who have been at MWC but I don't anticipate any changes as this is a fairly simple requirement.

Also, re Comment 4, there is a simple banner that appears for a few seconds as confirmation.

Let me know if you have any questions.

Comment 6

[[:fabrice] Fabrice Desré]

Attachment: patch v2

Now with a 3 seconds banner displaying error or success.

Comment 7

[[:fabrice] Fabrice Desré]

Now available also at https://github.com/mozilla-b2g/gaia/pull/8479

Comment 8

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/8479

Pointer to Github pull-request

Comment 9

[Ben Francis [:benfrancis]]

This is an awesome feature, thanks Fabrice! Now I can save my boarding pass to the gallery when I fly home :)

Just some cleanup work to do I think, see my comments on the pull request.

Maybe this could benefit from a security review as we're saving data from the world wild web onto device storage?

Do we need latel10n for 1.1 features?

Comment 10

[Ben Francis [:benfrancis]]

Comment on attachment 721619 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/8479

r+me with comments addressed (see comments in pull request).

Comment 11

[[:fabrice] Fabrice Desré]

Ben, I updated the pull request to address your comments and Kevin's. I also fixed the issue with saving twice an image with the same name. It's now prepending the file name with the current date ms and even allows retries.

Comment 12

[Ben Francis [:benfrancis]]

This looks great now, thanks! It can't currently be applied cleanly, maybe while you merge with upstream you could rebase and squash into one commit before merging :)

Thanks Fabrice

Comment 13

[[:fabrice] Fabrice Desré]

Pushed on master:
https://github.com/mozilla-b2g/gaia/commit/1b1fb4a6637e6cd1254d419057bbe6875d52770c

Comment 14

[[:fabrice] Fabrice Desré]

and https://github.com/mozilla-b2g/gaia/commit/cedc21afea33ba230b75debdb251ea4b5931ac96 because the linter doesn't like all the good things like double quotes!

Comment 15

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1-train.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x  1b1fb4a6637e6cd1254d419057bbe6875d52770c
  <RESOLVE MERGE CONFLICTS>
  git commit
  git checkout v1-train
  git cherry-pick -x  cedc21afea33ba230b75debdb251ea4b5931ac96
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 16

[[:fabrice] Fabrice Desré]

It has no dependencies that I know of.

Comment 17

[John Ford [:jhford] CET/CEST Berlin Time]

(In reply to Fabrice Desré [:fabrice] from comment #16)
> It has no dependencies that I know of.

in that case, could you please resolve the merge conflict?

Comment 18

[[:fabrice] Fabrice Desré]

pushed:
commit 777dc79736e081f24ebc413da5fc97ff2658407f

commit 1168b5f92e3262ea3c492105fa53a085bff6ce49

Comment 19

[John Ford [:jhford] CET/CEST Berlin Time]

jford2:~/b2g/uplifting/gaia $ git branch --contains 777dc79736e081f24ebc413da5fc97ff2658407f
  v1-train
jford2:~/b2g/uplifting/gaia $ git branch --contains 1168b5f92e3262ea3c492105fa53a085bff6ce49
  v1-train

Thanks for the merge conflict resolutions!

Comment 20

[Jason Smith [:jsmith]]

I believe we've got test case coverage in MozTrap for this, right?

Comment 21

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

#6889