Browse Source

Merge pull request #12059 from electron/add-tabbedwindow-warning

add warning when addTabbedWindow is called on self
shelley vohr 7 years ago
parent
commit
dfa1dc43df

+ 5 - 2
atom/browser/api/atom_api_browser_window.cc

@@ -1082,8 +1082,11 @@ void BrowserWindow::ToggleTabBar() {
   window_->ToggleTabBar();
 }
 
-void BrowserWindow::AddTabbedWindow(NativeWindow* window) {
-  window_->AddTabbedWindow(window);
+void BrowserWindow::AddTabbedWindow(NativeWindow* window,
+                                    mate::Arguments* args) {
+  const bool windowAdded = window_->AddTabbedWindow(window);
+  if (!windowAdded)
+    args->ThrowError("AddTabbedWindow cannot be called by a window on itself.");
 }
 
 void BrowserWindow::SetVibrancy(mate::Arguments* args) {

+ 1 - 1
atom/browser/api/atom_api_browser_window.h

@@ -243,7 +243,7 @@ class BrowserWindow : public mate::TrackableObject<BrowserWindow>,
   void MergeAllWindows();
   void MoveTabToNewWindow();
   void ToggleTabBar();
-  void AddTabbedWindow(NativeWindow* window);
+  void AddTabbedWindow(NativeWindow* window, mate::Arguments* args);
 
   void SetVibrancy(mate::Arguments* args);
   void SetTouchBar(const std::vector<mate::PersistentDictionary>& items);

+ 2 - 1
atom/browser/native_window.cc

@@ -336,7 +336,8 @@ void NativeWindow::MoveTabToNewWindow() {
 void NativeWindow::ToggleTabBar() {
 }
 
-void NativeWindow::AddTabbedWindow(NativeWindow* window) {
+bool NativeWindow::AddTabbedWindow(NativeWindow* window) {
+  return true;  // for non-Mac platforms
 }
 
 void NativeWindow::SetVibrancy(const std::string& filename) {

+ 1 - 1
atom/browser/native_window.h

@@ -192,7 +192,7 @@ class NativeWindow : public base::SupportsUserData,
   virtual void MergeAllWindows();
   virtual void MoveTabToNewWindow();
   virtual void ToggleTabBar();
-  virtual void AddTabbedWindow(NativeWindow* window);
+  virtual bool AddTabbedWindow(NativeWindow* window);
 
   // Webview APIs.
   virtual void FocusOnWebView();

+ 1 - 1
atom/browser/native_window_mac.h

@@ -109,7 +109,7 @@ class NativeWindowMac : public NativeWindow {
   void MergeAllWindows() override;
   void MoveTabToNewWindow() override;
   void ToggleTabBar() override;
-  void AddTabbedWindow(NativeWindow* window) override;
+  bool AddTabbedWindow(NativeWindow* window) override;
 
   void SetVibrancy(const std::string& type) override;
   void SetTouchBar(

+ 7 - 3
atom/browser/native_window_mac.mm

@@ -1671,10 +1671,14 @@ void NativeWindowMac::ToggleTabBar() {
   }
 }
 
-void NativeWindowMac::AddTabbedWindow(NativeWindow* window) {
-  if ([window_ respondsToSelector:@selector(addTabbedWindow:ordered:)]) {
-    [window_ addTabbedWindow:window->GetNativeWindow() ordered:NSWindowAbove];
+bool NativeWindowMac::AddTabbedWindow(NativeWindow* window) {
+  if (window_.get() == window->GetNativeWindow()) {
+    return false;
+  } else {
+    if ([window_ respondsToSelector:@selector(addTabbedWindow:ordered:)])
+      [window_ addTabbedWindow:window->GetNativeWindow() ordered:NSWindowAbove];
   }
+  return true;
 }
 
 void NativeWindowMac::SetRenderWidgetHostOpaque(bool opaque) {

+ 6 - 0
spec/api-browser-window-spec.js

@@ -733,6 +733,12 @@ describe('BrowserWindow module', () => {
         done()
       })
     })
+
+    it('throws when called on itself', () => {
+      assert.throws(() => {
+        w.addTabbedWindow(w)
+      }, /AddTabbedWindow cannot be called by a window on itself./)
+    })
   })
 
   describe('BrowserWindow.setVibrancy(type)', () => {