libsdl-org / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
9.91k stars 1.83k forks source link

SDL_ShowMessageBox can be maximized by the parent process #9434

Open learn-more opened 7 months ago

learn-more commented 7 months ago

On Windows, when starting a process, there are options that can be configured on the initial window. In a shortcut this looks like this: image

And when creating a process with CreateProcess, it can be specified in the PROCESS_INFORMATION in the fields wShowWindow and dwFlags.

This maximizes a window even if the messagebox is not created maximized. On a normal window (SDL_CreateWindow) this does not seem to occur anymore (in SDL3, in SDL2 a normal window was affected as well).

Example code that reproduces:

int main(int argc, char* argv[])
{
    SDL_MessageBoxButtonData buttons[2] = {};
    buttons[0].buttonID = 0;
    buttons[0].text = "Quit";
    buttons[1].buttonID = 1;
    buttons[1].text = "Maximized";

    SDL_MessageBoxData mbdata = {};
    mbdata.flags = SDL_MESSAGEBOX_INFORMATION;
    mbdata.message = "Spawn the next window 'maximized' or quit:";
    mbdata.title = "Example";
    mbdata.numbuttons = _countof(buttons);
    mbdata.buttons = buttons;
    int button = -1;
    SDL_ShowMessageBox(&mbdata, &button);
    if (button == 1)
    {
        char buf[512];
        GetModuleFileNameA(NULL, buf, _countof(buf));

        STARTUPINFOA si = { sizeof(si) };
        // The magic / bug:
        si.dwFlags = STARTF_USESHOWWINDOW;
        si.wShowWindow = SW_MAXIMIZE;
        PROCESS_INFORMATION pi = {};
        if (CreateProcessA(buf, NULL, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi))
        {
            CloseHandle(pi.hProcess);
            CloseHandle(pi.hThread);
        }
    }
    return 0;
}

Putty has a workaround for this: https://github.com/KasperDeng/putty/blob/037a4ccb6e731fafc4cc77c0d16f80552fd69dce/putty-src/windows/windlg.c#L662

Should there be something in SDL that works around it, or is that already happening but not triggering for the messagebox?

slouken commented 1 month ago

We are scoping work for the SDL 3.2.0 release, so please let us know if this is a showstopper for you.

learn-more commented 1 month ago

We are scoping work for the SDL 3.2.0 release, so please let us know if this is a showstopper for you.

I'll consider sticking on 2.x or moving to another library for this.

slouken commented 1 month ago

It looks like you've already investigated this and added a workaround. Can you turn that into a tested PR for SDL 3.2?

learn-more commented 4 weeks ago

It looks like you've already investigated this and added a workaround. Can you turn that into a tested PR for SDL 3.2?

I'll have a go at it.

slouken commented 4 weeks ago

Thanks!

learn-more commented 4 weeks ago

I created a pr with the testcase, could you please check that it follows the style required? I am not very familiar with sdl testcases, so I tried to copy the style of other tests.

The actual fix is not implemented yet, so the test fails for now.

slouken commented 4 weeks ago

I added a couple of comments, but that generally looks fine. Feel free to add the fix as a second commit on that PR.

learn-more commented 2 weeks ago

I added a couple of comments, but that generally looks fine. Feel free to add the fix as a second commit on that PR.

The fix has been added. I also wanted to test the other types of dialogs, but the task dialog that is normally used as dialogbox is spawned from a thread, which is incompatible with the hooking method I used. There are 4 options that I see to mitigate this:

  1. Move the SetWindowsHookEx to a dll that can hook system-wide
  2. Create a dll that applies the hook per-thread in DLL_THREAD_ATTACH (those callbacks are only delivered to a dll afaik, not to a process)
  3. Poll for open dialogs instead of using a hook.
  4. Try to toy around with thread_local stuff (maybe that can be used to 'detect' thread creation).

Which would be the preferred method?