Mats Palmgren (inactive) Reporter
	Description • 12 years ago

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

	Mats Palmgren (inactive) Reporter
	Comment 1 • 12 years ago

2012-08-17-03-05-55 works
2012-08-18-03-06-04 broken

Brian, bug 731307 seems to be in that range?

	Mats Palmgren (inactive) Reporter
	Comment 2 • 12 years ago

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.

	Robert Kaiser
	Comment 3 • 12 years ago

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

	Mats Palmgren (inactive) Reporter
	Comment 4 • 12 years ago

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

	Brian R. Bondy [:bbondy] Assignee
	Comment 5 • 12 years ago

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

	Brian R. Bondy [:bbondy] Assignee
	Comment 7 • 12 years ago

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

Tested on linux

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 8 • 12 years ago

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.

	Brian R. Bondy [:bbondy] Assignee
	Comment 9 • 12 years ago

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

	Brian R. Bondy [:bbondy] Assignee
	Comment 11 • 12 years ago

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.

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 12 • 12 years ago

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 :)

	Brian R. Bondy [:bbondy] Assignee
	Comment 13 • 12 years ago

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.

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 14 • 12 years ago

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+.

	Brian R. Bondy [:bbondy] Assignee
	Comment 15 • 12 years ago

np thanks for the feedback and suggestion.

	Brian R. Bondy [:bbondy] Assignee
	Comment 16 • 12 years ago

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

	neil@parkwaycc.co.uk
	Comment 17 • 12 years ago

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.

	Ryan VanderMeulen [:RyanVM]
	Comment 18 • 12 years ago

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

	Brian R. Bondy [:bbondy] Assignee
	Comment 19 • 12 years ago

(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,

	Brian R. Bondy [:bbondy] Assignee
	Comment 20 • 12 years ago

Comment 17 fixed in Bug 785744