Closed Bug 563962 Opened 10 years ago Closed 10 years ago

NS_ERROR_MALFORMED_URI nsIFilePicker.file in contentAreaUtils.js::getTargetFile in this case

Categories

(Firefox for Android Graveyard :: General, defect)

Fennec 1.1
x86
Windows 7
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: martijn.martijn, Assigned: mfinkle)

Details

(Keywords: testcase)

Attachments

(3 files, 2 obsolete files)

Attached file testcase
Make sure you have the javascript.options.showInConsole pref set to true and
the browser.console.showInPanel set to true in about:config.

To reproduce:
- Open testcase, click and hold the stylus on the broken image to invoke the context menu with "Save Image" and then click on that to save it.

This results in the javascript error, which I'll attach as a screenshot, shortly.
This only happens on the n900, I haven't seen this on windows at least.
Attached patch patch (obsolete) — Splinter Review
Borrow some code from Firefox. We now check to make sure the image is fully loaded.
Assignee: nobody → mark.finkle
Attachment #443650 - Flags: review?(mbrubeck)
Comment on attachment 443650 [details] [diff] [review]
patch

>+  onCompletedImage: false

Fennec doesn't actually use onCompletedImage.  (Firefox uses it only for the "Reload Image" menu item.)  Should we just remove it?

>+      if (type.indexOf("image") != -1 && this.onImage && this.onLoadedImage) {

this.onImage is redundant and can be removed.

This limits the usefulness of contextmenu add-ons.  For example, this add-on would be especially useful for non-loaded images:
https://addons.mozilla.org/en-US/mobile/addon/157424/

Shouldn't this be configured per item, rather than for all type="image" items?  The same goes for onSaveableLink.
Attachment #443650 - Flags: review?(mbrubeck) → review-
Attached patch patch 2 (obsolete) — Splinter Review
* Removes unused onCompletedimage
* Removes redundant onImage and onLink checks
* Adds isVoice, onVoiceLink and "callto" link type so add-ons can support voice links
* Changes "Save Image" to a "image-loaded" type
Attachment #443650 - Attachment is obsolete: true
Attachment #443682 - Flags: review?(mbrubeck)
Comment on attachment 443682 [details] [diff] [review]
patch 2

>       if (type.indexOf("image") != -1 && this.onImage) {
>         first = (first ? first : command);
>         last = command;
>         command.hidden = false;
>         continue;
>+      } else if (type.indexOf("image-loaded") != -1 && this.onLoadedImage) {

This fails on the test case because "image-loaded".indexOf("image") != -1 (so it uses the first branch instead of continuing to the "else if").

We could either use type.split(/\W/) and match whole strings, or do things in this order:

if (type.indexOf("image-loaded")) {
  if (this.onLoadedImage) {
    // ...
  }
} else if (type.indexOf("image") != -1 && this.onImage) {
  // ...
}

The rest of the patch looks good.
Attachment #443682 - Flags: review?(mbrubeck) → review-
Attached patch patch 3Splinter Review
* Uses .split(/\s+/) to create an array from the space delimited types
Attachment #443682 - Attachment is obsolete: true
Attachment #443705 - Flags: review?(mbrubeck)
Attachment #443705 - Flags: review?(mbrubeck) → review+
I'm seeing a similar error when trying to use "Save As PDF" on about:* pages or with a "Problem loading page" page. Will the patch fix that too?
(In reply to comment #8)
> I'm seeing a similar error when trying to use "Save As PDF" on about:* pages or
> with a "Problem loading page" page. Will the patch fix that too?

Nope. different bug.
pushed m-b:
http://hg.mozilla.org/mobile-browser/rev/26c26eb84a51

pushed m-1.1:
http://hg.mozilla.org/releases/mobile-1.1/rev/bc9640b25b80
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
(In reply to comment #9)
> (In reply to comment #8)
> > I'm seeing a similar error when trying to use "Save As PDF" on about:* pages or
> > with a "Problem loading page" page. Will the patch fix that too?
> 
> Nope. different bug.

Ok, I filed bug 564035 for that.
verified FIXED on builds:

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2.5pre) Gecko/20100506 Namoroka/3.6.5pre Fennec/1.1b2pre

and

Mozilla/5.0 (X11; U; Linux armv71; Nokia N900; en-US; rv:1.9.3a5pre) Gecko/20100506 Namoroka/3.7a5pre Fennec/2.0a1pre
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.