wxFrame.Reparent merges the reparented window on the parent on Windows and Mac

[wxtrac]

Issue migrated from trac ticket # 18785

component: wxMSW | priority: normal | resolution: fixed

2020-06-08 21:30:19: carandraug created the issue

---

When reparenting one frame to another, the content of the child frame is merged into the new parent frame instead of staying as two separate frames. This issue seems to happen on windows and mac. It behaves properly under GTK3.

Here's a minimal example:

#include <wx/wx.h>

class App : public wxApp
{
public:
    virtual bool OnInit();
};

IMPLEMENT_APP(App)

bool App::OnInit()
{
  wxFrame* frame1 = new wxFrame (NULL, wxID_ANY, "frame 1");
  frame1->Show();
  wxFrame* frame2 = new wxFrame (NULL, wxID_ANY, "frame 2");
  frame2->Reparent(frame1);
  frame2->Show();
  frame1->Show(true);
  return true;
}

The use case is a program with multiple windows. One of those window is the main window which when closed should close the program. However, the main window is not the first to be created, so those created before the main window need to be reparented to the main window after the main window is created. one other window is created first to show progress and any messages during initialization. That window is kept afterwards.

I have found #18489 but I'm unsure if it's the same since it is not clear at all what the issue there was, only that it relates to Reparent.

[wxtrac]

2020-06-10 16:09:56: @vadz changed status from new to confirmed

2020-06-10 16:09:56: @vadz changed component from GUI-generic to wxMSW

2020-06-10 16:09:56: @vadz commented

Oops, yes, this is embarrassing because I do know about this problem, but somehow never thought that it existed in wxMSW.

Anyhow, the reason for it is that we set the parent of a TLW in wxMSW wxWindow::Reparent(), but we really should be setting the owner, which is quite different (as usual, Raymond Chen provides the best explanation of the difference). Confusingly enough, setting the owner is done using ::SetWindowLong(GWL_HWNDPARENT), but this is just Win32 API for you.

So the solution is to override Reparent() in wxNonOwnedWindow in wxMSW to use this function instead of ::SetParent(). It should be pretty simple to do and if you can do it and test that it works, please don't hesitate to [HowToSubmitPatches submit a PR or a patch].

P.S. I have no idea about Mac unfortunately.

[wxtrac]

2020-06-10 16:29:56: carandraug commented

---

I won't be able to test it on Windows or Mac.

To be honest, no one ever ran that minimal C++ example in Windows or Mac when I reported it. This all comes from a wxPython issue reported to me by both Mac and Windows users, and in both cases confirmed with a Python version of the minimal example. I only have Linux where this issue does not happen.

[wxtrac]

2020-06-12 11:19:53: dstoy (dstoy) uploaded file reparenting_msw.patch (1.0 KiB)

[wxtrac]

2020-06-12 11:21:29: dstoy (dstoy) commented

---

Hi, colleague of carandraug here. I was able to do some testing on Windows and I attached a patch to the ticket, using the solution suggested by vadz.

[wxtrac]

2020-06-13 13:38:18: @vadz commented

---

Thanks for the patch, but this is not really my suggestion as it uses SetWindowLongPtr(GWLP_HWNDPARENT) for all windows, not just top level ones. And this is wrong, MSDN explicitly says:

Do not call SetWindowLongPtr with the GWLP_HWNDPARENT index to change the parent of a child window. Instead, use the SetParent function.

We really need to override Reparent() and do a different thing for TLWs.

[wxtrac]

2020-06-20 21:09:02: dstoy (dstoy) uploaded file reparenting_msw_v2.patch (4.9 KiB)

[wxtrac]

2020-06-20 21:09:31: dstoy (dstoy) commented

---

Thanks for the feedback.

After doing some reading, I realise that on Windows wxFrame is a top-level window and as such it cannot have a parent, although it may have an owner. The parent argument of the contructor of wxFrame would assign the owner instead, but only if the wxFRAME_FLOAT_ON_PARENT style is present. So the example in this bug report will create both frames with neither a parent nor an owner. I don't know if this is intentional.

In any case, for top-level windows Reparent is sort of like "Reown". I know that Microsoft say not to use SetWindowLongPtr to change the parent but I am changing the owner instead. That's the subtle difference. I've submitted a revised patch for this. Let me know your comments.

[wxtrac]

2020-07-09 00:32:37: @vadz commented

---

Thanks, this does do what I proposed to do and seems to work, but it can be simplified because we don't need to play WS_EX_CONTROLPARENT games with the top level windows, they're not affected by IsDialogMessage() problems as it stops at the TLW boundary anyhow.

So I'm finally committing a more minimal version of your patch.

Thanks again!

[wxtrac]

2020-07-09 00:35:41: @vadz changed status from confirmed to closed

2020-07-09 00:35:41: @vadz set resolution to fixed

2020-07-09 00:35:41: @vadz commented

In 5e7e89de16bb5d184d51e2f93924a2b66f9896a1: Fix re-parenting TLWs in wxMSW

We need to set the new owner for the TLW, instead of using the new parent as the actual parent, in the MSW sense, as this results in a weird situation in which the TLW becomes a child (i.e. non-TLW) window.

Closes #18785.