784842 - Exception... "'JavaScript component does not have a method named: "open"' when calling method: [nsIFilePicker::open]"

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Mats Palmgren (inactive)]

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 (inactive)]

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 (inactive)]

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]

AFAIK, many Linux distros set this pref that way by default.

Comment 4

[Mats Palmgren (inactive)]

Oops, I mixed up true/false in comment 2.  Hopefully it's clear what
I meant anyway.

Comment 5

[Brian R. Bondy [:bbondy]]

Didn't know that existed :) I'll fix that shortly.

Comment 6

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1

Comment 7

[Brian R. Bondy [:bbondy]]

Comment on attachment 654489 [details] [diff] [review]
Patch v1

Tested on linux

Comment 8

[:Gavin Sharp [email: [e-mail]]]

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]]

Oh cool, thanks for the info, I'll update it a bit later today.

Comment 10

[Brian R. Bondy [:bbondy]]

Attachment: Patch v2

Comment 11

[Brian R. Bondy [:bbondy]]

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: [e-mail]]]

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]]

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: [e-mail]]]

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]]

np thanks for the feedback and suggestion.

Comment 16

[Brian R. Bondy [:bbondy]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/730c4f0ec3ae

Comment 17

[[e-mail]]

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]]

https://hg.mozilla.org/mozilla-central/rev/730c4f0ec3ae

Comment 19

[Brian R. Bondy [:bbondy]]

(In reply to [e-mail] 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]]

Comment 17 fixed in Bug 785744