[haiku-development] Re: BAboutWindow lifecycle

  • From: John Scipione <jscipione@xxxxxxxxx>
  • To: "haiku-development@xxxxxxxxxxxxx" <haiku-development@xxxxxxxxxxxxx>
  • Date: Mon, 29 Apr 2013 14:58:27 -0400

On Mon, Apr 29, 2013 at 1:55 PM,  <pulkomandy@xxxxxxxxxxxxx> wrote:
> I have a problem with the way our BAboutWindow currently behaves. The idea
> of the current code is that you create the window on the first call to
> App::AboutRequested and show it. Instead of closing, the window is hidden,
> so that subsequent calls just show it again.
>
> To do this, the BAboutWindow code overrides QuitRequested to hide the
> window and return false (that stops the quitting process). An annoying side
> effect is that Application::QuitRequested will not quit the app if a window
> returns false from this method.
>
> Moreover, having the window hidden means its BLooper keeps running. This
> sounds like wasting more resources that recreating the window if it is
> needed again (an unlikely event, you don't usually look at an about window
> multiple times). It also is nonstandard behavior, and a trap for the
> unsuspecting developers using the class.
>
> This can easily be fixed by letting the window close. Callers in our source
> tree can be adjusted to always recreate it. There is a regression in doing
> that, however: we also had code to detect an already visible about window
> and bring it to the current workspace and/or activating it. I can see no
> easy way of doing that if the application does not keep a reference to the
> window, and since the window can quit itself, I don't see how the app could
> make sure the reference is still valid.
>
> I think a way to solve this is making the about window application-modal.
> This will make sure it is above other windows for the app and removes the
> need for explicitly activating it in useful cases (you could still manage
> to open multiple about windows using hey, but I don't think that's an
> actual problem). Having separate about window in different workspaces don't
> seem too much of a problem either (or maybe application-modality already
> solves that as well ?)

I really don't want the about box to be modal since that blocks the
app, but if you wanted to go that way it would be pretty easy to do
using BAlert as a base class instead of BWindow. Making the about box
modal blocks the calling app which is less than ideal.

The reason that I hid the about box when the window is closed instead
of destroying the object is because if I were to destroy the window
the reference to the about box in the calling app would then point to
freed memory and thus would crash the app next time the variable is
referenced say to check if it's NULL. I basically used hiding as a way
to take a shortcut around this problem.

But, there is a solution to this problem. The about box needs to send
a message back to the original app (and maybe wait for a reply just to
be sure) on close so that the calling app can set it's about window
reference to NULL.

Doing this in an application-agnostic way means that we'll need to
either add a system message constant for this activity like
B_QUIT_ABOUT_DIALOG (not ideal) or pass in the handler to send the
message back to in the About Box constructor so that it knows what
handler to send the message back to on close (better). We'll also have
to figure out some sort of unique message constant to use for this,
or, we'll have to also pass the message constant into the about box
constructor as well.

Unfortunately this is only a partial solution because it doesn't
address some of the other side effects we get from hiding instead of
destroying the window. For instance the about window will no longer
remember it's position but that functionality can be added on.

I had this discussion before here but maybe it's a good time to bring
it up again. If we want to remember the positions of each window we'll
need to create a settings file for each window in boot/home/config
somewhere. This seems less than ideal to me because the directory will
soon fill up with hundreds of settings files but I haven't heard a
better suggestion yet. This is really something that app_server should
handle.

> On a final note, I also plan to use BAboutWindow as the default behavior
> for BApplication::BAboutRequested, which currently opens a BAlert with just
> the application name in it. I think BAboutWindow will show good enough
> results (it gets the icon, application name, and other informations from
> the app resources).

+1

> I wanted to make sure it's ok to do these changes, as it will need
> reworking all BAboutWindow users in our source tree. Is everyone ok with
> these changes?

With the above caveats...

Other related posts: