708273 - Add "Save Image" to context menu on images in content

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P4)

Description

[Madhava Enros [:madhava]]

We should let people long tap on images and save them to the Android Gallery.

Comment 1

[Wesley Johnston (:wesj)]

Attachment: patch

Patch

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 582746 [details] [diff] [review]
patch


>diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js
>     init: function() {
>       this.textContext = this.SelectorContext("input[type='text'],input[type='password'],textarea");
>       this.linkContext = this.SelectorContext("a:not([href='']),area:not([href='']),link");
>+      this.imageContext = this.SelectorContext("img");
>       Services.obs.addObserver(this, "Gesture:LongPress", false);

Add a blank line before the Services line

>       // TODO: These should eventually move into more appropriate classes

Yes, they really should. ContextHelper or something. Sooner the better.

>                });
>+      this.add(Strings.browser.GetStringFromName("contextmenu.saveImage"),

Add a blank line separator

>+               function(aTarget) {

>+                 var contentDisposition = "";
>+                 var type = "";

let

>+                 var browser = BrowserApp.getBrowserForDocument(aTarget.ownerDocument);

let

>+                 ContentAreaUtils.internalSave(aTarget.currentURI.spec, null, null,
>+                                               contentDisposition,
>+                                               type,
>+                                               false, "SaveImageTitle",
>+                                               null, browser.documentURI, true, null);

Try one line (this ain't java!)

r+ with nits fixed

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 582746 [details] [diff] [review]
patch

changed the summary to something boring, but not incorrect

Comment 4

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a1a8c14cc2a4

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a1a8c14cc2a4

Comment 6

[Scoobidiver (away)]

It missed the Firefox 11 release for a few hours.

Comment 7

[Scoobidiver (away)]

May be I am wrong about the Firefox 11 target because this changeset is between:
* The latest changeset in 11.0a1/20111220: http://hg.mozilla.org/mozilla-central/rev/2afd7ae68e8b.
* The first changeset in 11.0a2/20111220: http://hg.mozilla.org/mozilla-central/rev/a8506ab2c654

Comment 8

[Matt Brubeck (:mbrubeck)]

(In reply to Scoobidiver from comment #6)
> It missed the Firefox 11 release for a few hours.

Nope, it made it into Aurora with the rest of the Firefox 11 merge: http://hg.mozilla.org/releases/mozilla-aurora/rev/a1a8c14cc2a4

Comment 9

[Camelia Urian]

Build ID: Mozilla/5.0 (Android; Linux armv7l; rv:11.0a2) Gecko/20111222 Firefox/11.0a2 Fennec/11.0a2
Mozilla/5.0 (Android; Linux armv7l; rv:12.0a1) Gecko/20111222 Firefox/12.0a1 Fennec/12.0a1
Device: Samsung Nexus S
OS: Android 2.3

Verified aurora ans nightly

Comment 10

[Camelia Urian]

Test case updated: https://litmus.mozilla.org/show_test.cgi?id=33638