Make sure target.currentURI is not null before saving an image

VERIFIED FIXED in Firefox 14

Status

()

VERIFIED FIXED
7 years ago
2 years ago

People

(Reporter: mfinkle, Assigned: mfinkle)

Tracking

unspecified
Firefox 15
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox14 verified, firefox15 verified, firefox16 verified, firefox17 verified, blocking-fennec1.0 +)

Details

Attachments

(1 attachment)

Duplicate of this bug: 750242
Created attachment 619561 [details] [diff] [review]
patch

This patch uses a more stringent selector logic to allow the image to be saved. The image must have a currentURI and must be loaded. This matches desktop and XUL fennec behavior:

http://mxr.mozilla.org/mozilla-central/source/mobile/xul/chrome/content/content.js#876
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js#521

The patch also removes a lookup for browser, since we already have access to the documentURIObject, like desktop:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js#1112

Tested and it works.
Assignee: nobody → mark.finkle
Attachment #619561 - Flags: review?(wjohnston)
Comment on attachment 619561 [details] [diff] [review]
patch

Review of attachment 619561 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/chrome/content/browser.js
@@ +1188,5 @@
>                   try {
>                      String(props.get("content-disposition", Ci.nsISupportsCString));
>                      String(props.get("type", Ci.nsISupportsCString));
>                   } catch(ex) { }
> +                 ContentAreaUtils.internalSave(aTarget.currentURI.spec, null, null, contentDisposition, type, false, "SaveImageTitle", null, aTarget.ownerDocument.documentURIObject, true, null);

Aha! I've been wondering why this was wrong sometimes.
Attachment #619561 - Flags: review?(wjohnston) → review+
This patch defends against the crash in bug 741432
blocking-fennec1.0: --- → ?
https://hg.mozilla.org/mozilla-central/rev/76299fb5d499
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 15
Aurora nom?

Updated

7 years ago
Duplicate of this bug: 751179
Comment on attachment 619561 [details] [diff] [review]
patch

[Approval Request Comment]
Regression caused by (bug #): native rewrite
User impact if declined: potential crash when trying to save an image
Testing completed (on m-c, etc.): dev testing and it's on m-c
Risk to taking this patch (and alternatives if risky):potential crash
String changes made by this patch: none
Attachment #619561 - Flags: approval-mozilla-aurora?

Updated

7 years ago
Blocks: 741432
Attachment #619561 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/0f21b590e841
status-firefox14: --- → fixed
status-firefox15: --- → fixed
blocking-fennec1.0: ? → +
Status: RESOLVED → VERIFIED
status-firefox14: fixed → verified
status-firefox15: fixed → verified
status-firefox16: --- → verified
status-firefox17: --- → verified
You need to log in before you can comment on or make changes to this bug.