252058 - filepicker with null displayDirectory crashes show() on Windows and OS/2, works on Linux - Trunk [@ nsFilePicker::ShowW]

Status: VERIFIED DUPLICATE of bug 268311

(Core :: XUL, defect)

Description

[Daniel Barrett]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.1) Gecko/20040707
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.1) Gecko/20040707

This code works on Linux but crashes Mozilla 1.7 in Windows XP:

var nsIFilePicker = Components.interfaces.nsIFilePicker;
var fp =
Components.classes["@mozilla.org/filepicker;1"].createInstance(nsIFilePicker);
fp.init(window, "my title", nsIFilePicker.modeGetFolder);
fp.displayDirectory = null; // this causes the problem
// Windows crash happens on the next line
var res = fp.show();


Reproducible: Always
Steps to Reproduce:
1. Get "UniversalXPConnect" privileges. 
2. Run the code above.

Actual Results:  
Mozilla crash and exit.

Expected Results:  
If the null is acceptable, open the filepicker. If not, throw an appropriate
exception.

Comment 1

[timeless]

this sounds familiar, didn't we have this problem on trunk for a bit?

Comment 2

[[e-mail]]

iirc darin tripped over this with his ftp upload patch.

Comment 3

[Mike Kaply [:mkaply]]

The problem here is that mDisplayDirectory is created in the constructor, and
then assumed to be there when it is used in Show.

If a SetDisplayDirectory comes in and wipes it out, we crash.

Instead of initializing mDisplayDirectory in the constructor, maybe we should
initialize it at the beginning of show if it is null?

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

fwiw, this is the number 6 trunk topcrash, according to
http://talkback-public.mozilla.org/reports/mozilla/Trunk/Trunk-topcrashers.html

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

although that topcrash may be a regression; user comments say it's from save
link target as. seems to have appeared on 101107 - two days after Bug 160454, if
I'm reading this date format right. hm... the code doesn't really look like this
part has changed, though...

Comment 6

[Mike Kaply [:mkaply]]

We definitely should just fix this once and for all.

Can someone give me a real JS component I can use to recreate?

thanks

Comment 7

[Phil]

(In reply to comment #5)
> although that topcrash may be a regression; user comments say it's from save
> link target as. seems to have appeared on 101107 - two days after Bug 
160454, if
> I'm reading this date format right. hm... the code doesn't really look like 
this
> part has changed, though...

Does seem to have broken (presumeably with bug 160454) so will look into it.

Comment 8

[Jay Patel [:jay]]

Latest Talkback data suggests this crash might have gone away after 11/8
MozillaTrunk builds (bug 267406 was fixed that day).  Does anyone know if this
particular crash is still around in any form?  Or should we mark this fixed?

Comment 9

[Boris Zbarsky [:bzbarsky]]

> Does anyone know if this particular crash is still around in any form?

Yes.  That bug checked in a workaround for this bug in the save as code (it no
longer sets displayDirectory to null).

This bug still needs to be fixed in the relevant toolkits.

Comment 10

[Phil]

(In reply to comment #9)
> This bug still needs to be fixed in the relevant toolkits.

What is your opinion of comment #3 "...maybe we should initialize it at the
beginning of show if it is null?"

... or have the show method assert where appropriate?

Comment 11

[Boris Zbarsky [:bzbarsky]]

I don't have any opinions on fixing this; the toolkits involved are not ones I'm
particularly familiar with.

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

looks like this was fixed in bug 268311?

*** This bug has been marked as a duplicate of 268311 ***