Closed Bug 708273 Opened 8 years ago Closed 8 years ago

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

Categories

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

11 Branch
ARM
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 11
Tracking Status
firefox11 --- verified
firefox12 --- verified

People

(Reporter: madhava, Assigned: wesj)

Details

(Whiteboard: [inbound][QA!])

Attachments

(1 file)

We should let people long tap on images and save them to the Android Gallery.
Priority: -- → P4
OS: Mac OS X → Android
Hardware: x86 → ARM
Attached patch patchSplinter Review
Patch
Assignee: nobody → wjohnston
Attachment #582746 - Flags: review?(mark.finkle)
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
Attachment #582746 - Flags: review?(mark.finkle) → review+
Comment on attachment 582746 [details] [diff] [review]
patch

changed the summary to something boring, but not incorrect
Attachment #582746 - Attachment description: Widget Patch 2/3 - Prevent Default stuff → patch
Flags: in-litmus?(fennec)
Whiteboard: [inbound] → [inbound][QA+]
https://hg.mozilla.org/mozilla-central/rev/a1a8c14cc2a4
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
It missed the Firefox 11 release for a few hours.
Version: unspecified → Firefox 12
Whiteboard: [inbound][QA+] → [inbound][QA+][fennec-aurora]
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
(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
Target Milestone: --- → Firefox 11
Version: Firefox 12 → Firefox 11
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
Status: RESOLVED → VERIFIED
Whiteboard: [inbound][QA+][fennec-aurora] → [inbound][QA+][veified fennec-aurora]
Whiteboard: [inbound][QA+][veified fennec-aurora] → [inbound][QA+][verified fennec-aurora]
Whiteboard: [inbound][QA+][verified fennec-aurora] → [inbound][QA+]
Test case updated: https://litmus.mozilla.org/show_test.cgi?id=33638
Flags: in-litmus?(fennec) → in-litmus+
Whiteboard: [inbound][QA+] → [inbound][QA!]
You need to log in before you can comment on or make changes to this bug.