When BeOS native BFilePanel is called from Menu->Open File, it blocks both access to browser and its repainting. Until BFilePanel is closed. dunno yet, what to revise - nsFilePickerBeOS or "general" XPFE menu code

This is true of the Save As panel as well, as you might expect. I agree it's pretty lame and annoying.

one way to fix this would be to use the xp panel, instead of the BFilePanel. then mozilla would control everything, as we all know it likes to do :)

I have seen this under Linux as well. Try changing the print command from lpr to kprinter (if running kde) to get the KDE print dialog. When it is run, all drawing within mozilla is stopped, until the dialog is closed. btw, I don't think we should go to the XPFileDialog

This problem is defferent form linux one, and "shouldn'be" - look at other BeOS apps using FilePanel. Problem is that RefsReceived message processing is done inside BFilePanel derived class itself, insted processing it somewhere outside, e.g. nsWindowBeOS.cpp code. And it needs usage of semaphores to block all other events until FilePanel gets this message (or cancel). Anyway, this problem isn't so important, but still may leave in TODO_IN_FUTURE list

Moving todo for Firefox 3.0 (unfortunately).

I've got an idea on how to solve this. Assigning it to me as a reminder.

Added a concept patch. This patch would in theory solve the problem, it's a solution the BeBook also mentions, however, it doesn't work. I think that's due to the fact that the parent offered in InitNative() isn't really the parent, but rather one of the ghost windows. Anyway, I need some help by the experts to continue this search.

This patch _should_ work, but it doesn't entirely. The problem shows up when trying to pick a file in the browser window itself. Picking files from the File menu and then Open File, and picking files from Browse windows in the preferences work without any issues, but from pages (such as bugzilla), the nsIAppshell associated with the nsIWidget is null. So basically, the issue revolves on whether it's predictable when the appshell is null, and where to get it if that's the case.

Here's another iteration. This one works! I thought I'd share it with the world. I'm going to do another one. Sergei pointed out that there is a better fix, which I'll try out in a moment.

This is another iteration, which works, however, after opening a file picker it becomes very slow for some reason. I really don't know what the reason is. I do know that this version is stylistically (when I tailor it a bit more) better than the third iteration.

Just a thought on why #4 might not work as espected. Perhaps the do_GetService creates a new appshell. In this process, the old port is deleted, thus discarding all the messages waiting there. A new one is created, but this one misses some important messages that slows down the rest of the app. Is this plausible?

It shouldn't create another I think. Will read code tomorrow.

Hi there, it's been a while. This weekend I dug up my old computer which contained the firefox source. Just to let you know how things have been going: I'm pretty sure that attachment 244713 [details] [diff] [review] is _the_ way to fix this issue in Firefox 2. There are two issues: 1. If the file dialog is being dragged, it still leaves drag traces in the firefox window, though as soon as you release the mousebutton it will redraw. 2. If the file picker is initiated from a web page, Firefox crashes. I came to the conclusion that this is because we request the AppShell, which is a native service in the browser itself, but it's probably 'locked off' when it is called from a web page (which seems wise). Solutions to 2: a) Don't redraw the window when there is no appshell. b) Dig deeper and find a way to get the pointer anyway. Perhaps there's an option to force retrieving the appshell. I don't know about this though. Niels

(In reply to comment #13) > Hi there, it's been a while. I think problem lies somewhere else, though. When you try to open file dialog window, there is loop of calls for nsAppShell::GetNativeEvent/DispatchNativeEvent starting to spin. While for "regular" moz-windows/widget we don't hook there (AppShell "sits" in ::Run() method loop in this case). But problem is that our GetNativeEvent is stub atm - no real code there. And code in DispatchNativeEvent is somewhat copycat of code in Run() - which don't look best solution for this case.

I solved here this problem, mathing way used by XUL modal windows. It probably needs bit updated nsAppShell.cpp for BeOS, but may work with existing one too. Semaphores in FilePicker WaitForSelection are replaced by while() loop in ::Show() method, at place where we call WaitForSelection. Code in Show() before and Loop looks like that: nsCOMPtr<nsIAppShell> mAppShell(do_CreateInstance(kAppShellCID)); NS_ENSURE_TRUE(mAppShell, NS_ERROR_FAILURE); mAppShell->Create(0, nsnull); mAppShell->Spinup(); ---------------------- ppanel->Show(); while(!ppanel->WaitForSelection()) { void* data; PRBool isRealEvent; PRBool processEvent; nsresult rv = NS_OK; rv = mAppShell->GetNativeEvent(isRealEvent, data); if(NS_SUCCEEDED(rv)) { // window->ModalEventFilter(isRealEvent, data, &processEvent); // if(processEvent) mAppShell->DispatchNativeEvent(isRealEvent, data); } }; and BFilePanel::WaitForSelection is simple bool now: bool nsFilePanelBeOS::WaitForSelection() { return mIsSelected; }

I tested this code in 1.8 branch and it seems to work. Opening the native file picker panel no longer freezes the main window behind it. I made no changes to nsAppShell.

(In reply to comment #16) > I tested this code in 1.8 branch and it seems to work. Opening the native file > picker panel no longer freezes the main window behind it. I made no changes to > nsAppShell. > That's nice. So we can work on this bug and appshell perfection separately

Do you want me to continue working on this? I might create some time for it, if you want to. (Though I won't mind you taking it up either).

(In reply to comment #18) > Do you want me to continue working on this? I might create some time for it, if > you want to. (Though I won't mind you taking it up either). > As this exact issue is heavily related to problems I'm investigating for moment, I prefer to take this one. But I will be grateful, if you, after we fix this bug, will look onto other filepicker problems. E.g. we lack additional but essential elements in our filepicker - like drop-down list with 3 versions how to save web-page - it needs additions to standard BFilePanel, and also filters - by extensions and by mime.

Okay, I'm sure you'll be able to perfect this patch. I'm glad the work I've put in this will get a good use. And if I've got some more time and I feel like working on Firefox, I'll have a look at the other issues regarding the filepicker.

(In reply to comment #20) > Okay, I'm sure you'll be able to perfect this patch. > > I'm glad the work I've put in this will get a good use. Nielx, btw, do you have idea, what for our file picker acquires semaphore here: http://lxr.mozilla.org/mozilla1.8/source/widget/src/beos/nsFilePicker.cpp#403 ? I removed all semaphore handling from code and seems working OK, but wondering if it has some deeper idea behind?:)

No, there's no deeper meaning. I have looked at some other code before devising this patch, and I'm pretty sure it's based on something someone else has done. I think that person chose to use semaphores because with acquire_sem_etc() you can set a timeout (so you don't have to snooze(...)), and it would also return immediately if the user selected an item. But yes, this can be easily implemented not using a semaphore.

allows proper update of parent window. Doug, if it is ok for you, set approval request after reviewing

Comment on attachment 294856 [details] [diff] [review] patch Code works as expected - resolves bug. Patch applies cleanly but needs additional change to nsFilePicker.h

Comment on attachment 294856 [details] [diff] [review] patch no r for incomplete patches

Comment on attachment 296417 [details] [diff] [review] additional changes needed for Sergei's patch to work. no need for subclassing picker as Blooper, will prepare proper patch this weekend, thanks for your efforts

(In reply to comment #26) > (From update of attachment 294856 [details] [diff] [review]) > no r for incomplete patches > thank you for clarifying policy. I was trying to move things along but understand now, the entire patch must be present to receive a "+"

> thank you for clarifying policy. I was trying to move things along but > understand now, the entire patch must be present to receive a "+" actually I'm more worried now about another patch - for native type recognition bug. As second review for that is stalled for some reason. As it touches code outside BeOS-only parts, we need that look from outside there, while bugs in widget/src/beos are totally in our responsibility (except approvals, yeah)

Doug, please test if this one works as expected

(In reply to comment #30) > Created an attachment (id=296685) [details] > complete patch > > Doug, please test if this one works as expected > This patch applies cleanly to 1.8 branch and works as expected. w00t! Patch does not apply cleanly to trunk - the nsFilePicker versions appear to be slightly different; no way to test trunk for now, either because of other build issues. This would be a good patch to add to 1.8 branch for Firefox 2.0.0.12.

Comment on attachment 296685 [details] [diff] [review] complete patch r=? set also approval request, when reviewed

Comment on attachment 296685 [details] [diff] [review] complete patch a=?

Comment on attachment 296685 [details] [diff] [review] complete patch Approved for the 1.8 branch; a=ss for release drivers. Please land as soon as possible.

Checking in mozilla/widget/src/beos/nsFilePicker.cpp; /cvsroot/mozilla/widget/src/beos/nsFilePicker.cpp,v <-- nsFilePicker.cpp new revision: 1.28.12.2; previous revision: 1.28.12.1 done Checking in mozilla/widget/src/beos/nsFilePicker.h; /cvsroot/mozilla/widget/src/beos/nsFilePicker.h,v <-- nsFilePicker.h new revision: 1.12.12.1; previous revision: 1.12 done

Thank you, Sergei!

Sergei, could you help us verify this bug fix using the release candidate for Fx 2.0.0.12 on BeOS?

(In reply to comment #37) > Sergei, could you help us verify this bug fix using the release candidate for > Fx 2.0.0.12 on BeOS? > Verified fixed in 2.0.0.12pre built on 2007-01-27.

Not really sure if this was FIXED or not, but BeOS is dead.