177720 - BeZilla FilePanel doesn't remember last visited (for Save) folder

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Sergei Dolgov]

solution  - remove mistakenly put (from beginning )parenthesises in FilePanel code.
Patch will follow

Comment 1

[Sergei Dolgov]

Attachment: Patch

Remembers last directory. Both for Open or Save.
P.S. for Paul's testing: but only if "Save" or "Open" button on file panel was
pressed ( if(result == PR_TRUE))

Comment 2

[Sergei Dolgov]

Paul, about crash you reported in private - it never happened here, but i used
this patch with older Mozilla sources only (up to 1.2).

Recently people reported numerous crashes with file picker even WITHOUT this
patch, e.g. http://bugzilla.mozilla.org/show_bug.cgi?id=185946 and i got
additional crash-reports on BeShare.

So i'll investigate this problem, but it needs updating of my mozilla tree.

Comment 3

[Sergei Dolgov]

Attachment: Patch with fallback

Added falback for non-existing folder name.

Comment 4

[Paul]

Comment on attachment 115169 [details] [diff] [review]
Patch with fallback

Well, my system still crashes after applying this patch and trying to open a
filepicker.  I am doing a full rebuild of my tree, and will test again once it
is done.

Comment 5

[Sergei Dolgov]

reassigning to BeOS-port developer

Comment 6

[Daniel Furrer]

Any news on this? (sorry for spamming)
Would it be helpful if I would do a build with this patch and test if it works?

Comment 7

[Paul]

Well, I found out why this crashes on my box, since today, I was able to build
with this patch applied, and no crash.  The difference is that today, I had
DEBUG disabled.  Which, leads me to the question:  Sergei, do you build mozilla
with DEBUG enabled?

Comment 8

[Sergei Dolgov]

No, i don't. Did it only once, but no HDD room now.

Comment 9

[Paul]

I don't know what the policy is, if a patch crashes under DEBUG, but not under a
normal build.  Chris? Timeless?

Comment 10

[timeless]

there's no policy prohibiting it.
but you probably should check it in with a line like:
//XXX bug ##### this crashes in debug builds
you still of course need whatever reviews are required.

could someone possibly post a stack and whatever interesting things they can for
the debug case so people can consider the problem?

Comment 11

[Paul]

I would post a stack crawl, if I could.  The problem is, when this thing
crashes, on both of my dev boxes, it does not just crash the app, but, the
entire OS.  Sometimes taking me to KDL, sometimes, just rebooting.  I need to
rebuild my dev branch w/ DEBUG enabled again, so I will give it a try, but won't
promise anything.

Comment 12

[Sergei Dolgov]

Testing now from debug build (running from mozilla/dist/bin). No crashes yet,
not application neither OS.

Comment 13

[Paul]

Comment on attachment 115169 [details] [diff] [review]
Patch with fallback

Well, I just did a full re-build, with this patch applied and debug enabled,
and, low and behold, it crashed again.	Sergei, did you rebuild/reconfigure
your entire tree when you tried it?
Either way, it seems to work just fine under non-debug builds, which, I guess
is "ok".  I just wish I knew why it was happenning.
I would like to research it a bit more before I approve the patch.

Comment 14

[Paul]

Comment on attachment 115169 [details] [diff] [review]
Patch with fallback

Sergei, if you can create a new patch, with a comment as per timeless's
suggestion, I'll approve it.

However, once this patch is applied, we should create a new bug, indicating
that the FilePanel crashes under DEBUG builds, so that we can track it, and,
hopefully get a stack crawl, if possible.

Comment 15

[Sergei Dolgov]

Attachment: Patch with notice

Comments added, new bug should be opened if we meet crash in DEBUG builds
again.

Comment 16

[Sergei Dolgov]

Comment on attachment 120690 [details] [diff] [review]
Patch with notice

review request

Comment 17

[Paul]

Attachment: taking it one step further

Taking this one step further, so that my debug builds are usable, i.e. don't
crash

Comment 18

[Paul]

Comment on attachment 120690 [details] [diff] [review]
Patch with notice

updated the patch

Comment 19

[Sergei Dolgov]

Wondering if it is checked in tree or not?

Comment 20

[Sergei Dolgov]

Comment on attachment 120786 [details] [diff] [review]
taking it one step further

r=[e-mail]

sr don't required, pure port patch

Comment 21

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

Checking in nsFilePicker.cpp;
/cvsroot/mozilla/widget/src/beos/nsFilePicker.cpp,v  <--  nsFilePicker.cpp
new revision: 1.23; previous revision: 1.22
done
Checking in nsFilePicker.h;
/cvsroot/mozilla/widget/src/beos/nsFilePicker.h,v  <--  nsFilePicker.h
new revision: 1.8; previous revision: 1.7
done