Last Comment Bug 784842 - Exception... "'JavaScript component does not have a method named: "open"' when calling method: [nsIFilePicker::open]"
: Exception... "'JavaScript component does not have a method named: "open"' whe...
Status: RESOLVED FIXED
: regression
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: x86_64 Linux
: -- major (vote)
: Firefox 17
Assigned To: Brian R. Bondy [:bbondy]
:
Mentors:
Depends on:
Blocks: 731307
  Show dependency treegraph
 
Reported: 2012-08-22 15:22 PDT by Mats Palmgren (:mats)
Modified: 2012-08-26 13:35 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (1.37 KB, patch)
2012-08-22 18:49 PDT, Brian R. Bondy [:bbondy]
no flags Details | Diff | Review
Patch v2 (1.32 KB, patch)
2012-08-23 18:36 PDT, Brian R. Bondy [:bbondy]
netzen: review+
gavin.sharp: feedback+
Details | Diff | Review

Description Mats Palmgren (:mats) 2012-08-22 15:22:49 PDT
This bug only occurs in my default profile, it works fine
in a fresh profile... but still, it seems something broke
in the past few days...

STEPS TO REPRODUCE
1. go to a bug in Bugzilla, such as this one
2. click "Add an attachment"
3. click "Browse"

ACTUAL RESULTS
Error: [Exception... "'JavaScript component does not have a method named: "open"' when calling method: [nsIFilePicker::open]"  nsresult: "0x80570030 (NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED)"  location: "native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0"  data: no]

PLATFORMS AND BUILDS TESTED
Bug occurs in Nightly 2012-08-22 on Linux64
Comment 1 Mats Palmgren (:mats) 2012-08-22 15:48:37 PDT
2012-08-17-03-05-55 works
2012-08-18-03-06-04 broken

Brian, bug 731307 seems to be in that range?
Comment 2 Mats Palmgren (:mats) 2012-08-22 16:01:30 PDT
Resetting ui.allow_platform_file_picker to false fixes it.
I had it set to true because I don't like the GTK file picker.
Comment 3 Robert Kaiser (not working on stability any more) 2012-08-22 16:02:46 PDT
AFAIK, many Linux distros set this pref that way by default.
Comment 4 Mats Palmgren (:mats) 2012-08-22 16:12:28 PDT
Oops, I mixed up true/false in comment 2.  Hopefully it's clear what
I meant anyway.
Comment 5 Brian R. Bondy [:bbondy] 2012-08-22 18:46:00 PDT
Didn't know that existed :) I'll fix that shortly.
Comment 6 Brian R. Bondy [:bbondy] 2012-08-22 18:49:03 PDT
Created attachment 654489 [details] [diff] [review]
Patch v1
Comment 7 Brian R. Bondy [:bbondy] 2012-08-22 20:09:28 PDT
Comment on attachment 654489 [details] [diff] [review]
Patch v1

Tested on linux
Comment 8 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-08-23 10:27:42 PDT
You can pass a JS function object directly to dispatch (no need for the object with the "run" property), since nsIRunnable is a [function]-annotated interface.
Comment 9 Brian R. Bondy [:bbondy] 2012-08-23 10:39:29 PDT
Oh cool, thanks for the info, I'll update it a bit later today.
Comment 10 Brian R. Bondy [:bbondy] 2012-08-23 18:36:51 PDT
Created attachment 654887 [details] [diff] [review]
Patch v2
Comment 11 Brian R. Bondy [:bbondy] 2012-08-24 04:40:50 PDT
Comment on attachment 654887 [details] [diff] [review]
Patch v2

Updated patch to not use an object with a run property and instead use a function directly.  Tested on linux with the perf set and it's working.
Comment 12 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-08-24 07:48:21 PDT
Comment on attachment 654887 [details] [diff] [review]
Patch v2

That specific part looks fine, haven't reviewed the rest of the patch (I don't think you need re-review for this nit :)
Comment 13 Brian R. Bondy [:bbondy] 2012-08-24 07:49:39 PDT
Comment on attachment 654887 [details] [diff] [review]
Patch v2

OK thanks, carrying forward roc's r+.  There is no other part to this patch by the way.  The async filepicker method has already landed.
Comment 14 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-08-24 07:52:04 PDT
I know, but I'm on a phone and am not able to look up interface details or otherwise look into this enough to grant r+.
Comment 15 Brian R. Bondy [:bbondy] 2012-08-24 07:54:51 PDT
np thanks for the feedback and suggestion.
Comment 17 neil@parkwaycc.co.uk 2012-08-26 06:18:22 PDT
Comment on attachment 654887 [details] [diff] [review]
Patch v2

>+      try {
>+        let result = this.show();
>+        aFilePickerShownCallback.done(result);
>+      } catch(ex) {
>+        aFilePickerShownCallback.done(this.returnCancel);
>+      }
I'm not convinced that show() can actually throw; it's certainly got enough try/catch blocks of its own. On the other hand, the callback could throw, and in that case you would attempt to call the callback again. Now you try to pass returnCancel, but JavaScript doesn't inherit interfaces in the same way C++ does, so you needed to use nsIFilePicker.returnCancel, instead you end up passing undefined; I can't remember whether XPConnect throws or casts it to zero.
Comment 18 Ryan VanderMeulen [:RyanVM] 2012-08-26 13:06:36 PDT
https://hg.mozilla.org/mozilla-central/rev/730c4f0ec3ae
Comment 19 Brian R. Bondy [:bbondy] 2012-08-26 13:07:50 PDT
(In reply to neil@parkwaycc.co.uk from comment #17)
> Comment on attachment 654887 [details] [diff] [review]
> Patch v2
> 
> >+      try {
> >+        let result = this.show();
> >+        aFilePickerShownCallback.done(result);
> >+      } catch(ex) {
> >+        aFilePickerShownCallback.done(this.returnCancel);
> >+      }
> I'm not convinced that show() can actually throw; it's certainly got enough
> try/catch blocks of its own. On the other hand, the callback could throw,
> and in that case you would attempt to call the callback again. 

Good point I'll fix that along with your previous mention of NS_ENSURE_ARG_POINTER in a new bug since I already pushed this to m-i.

> Now you try
> to pass returnCancel, but JavaScript doesn't inherit interfaces in the same
> way C++ does, so you needed to use nsIFilePicker.returnCancel, instead you
> end up passing undefined; I can't remember whether XPConnect throws or casts
> it to zero.

Doh ya, this code was the same as the mockfilepicker, but it has this at the top which makes it work correctly: returnCancel: Ci.nsIFilePicker.returnCancel,
Comment 20 Brian R. Bondy [:bbondy] 2012-08-26 13:35:23 PDT
Comment 17 fixed in Bug 785744

Note You need to log in before you can comment on or make changes to this bug.