fix: make 'setParentWindow' compatible under Windows

electron · Nov 21, 2018 · 3ba7489 · 3ba7489
1 parent fb52fdc
commit 3ba7489
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 23 deletions.
diff --git a/atom/browser/api/atom_api_top_level_window.cc b/atom/browser/api/atom_api_top_level_window.cc
@@ -669,6 +669,7 @@ void TopLevelWindow::SetParentWindow(v8::Local<v8::Value> value,
     parent_window_.Reset();
     window_->SetParentWindow(nullptr);
   } else if (mate::ConvertFromV8(isolate(), value, &parent)) {
+    RemoveFromParentChildWindows();
     parent_window_.Reset(isolate(), value);
     window_->SetParentWindow(parent->window_.get());
     parent->child_windows_.Set(isolate(), weak_map_id(), GetWrapper());
@@ -1059,9 +1060,7 @@ void TopLevelWindow::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("setContentProtection", &TopLevelWindow::SetContentProtection)
       .SetMethod("setFocusable", &TopLevelWindow::SetFocusable)
       .SetMethod("setMenu", &TopLevelWindow::SetMenu)
-#if !defined(OS_WIN)
       .SetMethod("setParentWindow", &TopLevelWindow::SetParentWindow)
-#endif
       .SetMethod("setBrowserView", &TopLevelWindow::SetBrowserView)
       .SetMethod("getNativeWindowHandle",
                  &TopLevelWindow::GetNativeWindowHandle)

diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc
@@ -975,20 +975,27 @@ void NativeWindowViews::SetParentWindow(NativeWindow* parent) {
   XSetTransientForHint(
       xdisplay, GetAcceleratedWidget(),
       parent ? parent->GetAcceleratedWidget() : DefaultRootWindow(xdisplay));
-#elif defined(OS_WIN) && defined(DEBUG)
-  // Should work, but does not, it seems that the views toolkit doesn't support
-  // reparenting on desktop.
-  if (parent) {
-    ::SetParent(GetAcceleratedWidget(), parent->GetAcceleratedWidget());
-    views::Widget::ReparentNativeView(GetNativeWindow(),
-                                      parent->GetNativeWindow());
-    wm::AddTransientChild(parent->GetNativeWindow(), GetNativeWindow());
-  } else {
-    if (!GetNativeWindow()->parent())
-      return;
-    ::SetParent(GetAcceleratedWidget(), NULL);
-    views::Widget::ReparentNativeView(GetNativeWindow(), nullptr);
-    wm::RemoveTransientChild(GetNativeWindow()->parent(), GetNativeWindow());
+#elif defined(OS_WIN)
+  // To set parentship between windows into Windows is better to play with the
+  //  owner instead of the parent, as Windows natively seems to do if a parent
+  //  is specified at window creation time.
+  // For do this we must NOT use the ::SetParent function, instead we must use
+  //  the ::GetWindowLongPtr or ::SetWindowLongPtr functions with "nIndex" set
+  //  to "GWLP_HWNDPARENT" which actually means the window owner.
+  HWND hwndParent = parent ? parent->GetAcceleratedWidget() : NULL;
+  if (hwndParent ==
+      (HWND)::GetWindowLongPtr(GetAcceleratedWidget(), GWLP_HWNDPARENT))
+    return;
+  ::SetWindowLongPtr(GetAcceleratedWidget(), GWLP_HWNDPARENT,
+                     (LONG_PTR)hwndParent);
+  // Ensures the visibility
+  if (IsVisible()) {
+    WINDOWPLACEMENT wp;
+    wp.length = sizeof(WINDOWPLACEMENT);
+    ::GetWindowPlacement(GetAcceleratedWidget(), &wp);
+    ::ShowWindow(GetAcceleratedWidget(), SW_HIDE);
+    ::ShowWindow(GetAcceleratedWidget(), wp.showCmd);
+    ::BringWindowToTop(GetAcceleratedWidget());
   }
 #endif
 }

diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md
@@ -1504,7 +1504,7 @@ On Windows it calls SetWindowDisplayAffinity with `WDA_MONITOR`.
 
 Changes whether the window can be focused.
 
-#### `win.setParentWindow(parent)` _Linux_ _macOS_
+#### `win.setParentWindow(parent)`
 
 * `parent` BrowserWindow
 

diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js
@@ -2953,12 +2953,6 @@ describe('BrowserWindow module', () => {
     })
 
     describe('win.setParentWindow(parent)', () => {
-      before(function () {
-        if (process.platform === 'win32') {
-          this.skip()
-        }
-      })
-
       beforeEach(() => {
         if (c != null) c.destroy()
         c = new BrowserWindow({ show: false })