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

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

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

We never check for a null URI before saving the image:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#1197

Comment 2

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

Attachment: 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.

Comment 3

[Wesley Johnston (:wesj)]

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.

Comment 4

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

This patch defends against the crash in bug 741432

Comment 5

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

https://hg.mozilla.org/mozilla-central/rev/76299fb5d499

Comment 6

[Aaron Train [:aaronmt]]

Aurora nom?

Comment 8

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

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

Comment 9

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/0f21b590e841