fix: set window to null when no window is passed (#18240)

* fix: set window to null when no window is passed

* add new specs for dialog

* fix process blocking for showMessageBox

lib/browser/api/dialog.js

@@ -47,7 +47,11 @@ const checkAppInitialized = function () {
 const saveDialog = (sync, window, options) => {
   checkAppInitialized()
 
-  if (window && window.constructor !== BrowserWindow) options = window
+  if (window && window.constructor !== BrowserWindow) {
+    options = window
+    window = null
+  }
+
   if (options == null) options = { title: 'Save' }
 
   const {
@@ -74,7 +78,11 @@ const saveDialog = (sync, window, options) => {
 const openDialog = (sync, window, options) => {
   checkAppInitialized()
 
-  if (window && window.constructor !== BrowserWindow) options = window
+  if (window && window.constructor !== BrowserWindow) {
+    options = window
+    window = null
+  }
+
   if (options == null) {
     options = {
       title: 'Open',
@@ -115,7 +123,11 @@ const openDialog = (sync, window, options) => {
 const messageBox = (sync, window, options) => {
   checkAppInitialized()
 
-  if (window && window.constructor !== BrowserWindow) options = window
+  if (window && window.constructor !== BrowserWindow) {
+    options = window
+    window = null
+  }
+
   if (options == null) options = { type: 'none' }
 
   const messageBoxTypes = ['none', 'info', 'warning', 'error', 'question']

spec/api-dialog-spec.js

@@ -1,8 +1,29 @@
 const { expect } = require('chai')
-const { dialog } = require('electron').remote
+const { closeWindow } = require('./window-helpers')
+const { remote } = require('electron')
+const { BrowserWindow, dialog } = remote
+const isCI = remote.getGlobal('isCi')
 
 describe('dialog module', () => {
   describe('showOpenDialog', () => {
+    it('should not throw for valid cases', () => {
+      // Blocks the main process and can't be run in CI
+      if (isCI) return
+
+      let w
+
+      expect(() => {
+        dialog.showOpenDialog({ title: 'i am title' })
+      }).to.not.throw()
+
+      expect(() => {
+        w = new BrowserWindow()
+        dialog.showOpenDialog(w, { title: 'i am title' })
+      }).to.not.throw()
+
+      closeWindow(w).then(() => { w = null })
+    })
+
     it('throws errors when the options are invalid', () => {
       expect(() => {
         dialog.showOpenDialog({ properties: false })
@@ -27,6 +48,24 @@ describe('dialog module', () => {
   })
 
   describe('showSaveDialog', () => {
+    it('should not throw for valid cases', () => {
+      // Blocks the main process and can't be run in CI
+      if (isCI) return
+
+      let w
+
+      expect(() => {
+        dialog.showSaveDialog({ title: 'i am title' })
+      }).to.not.throw()
+
+      expect(() => {
+        w = new BrowserWindow()
+        dialog.showSaveDialog(w, { title: 'i am title' })
+      }).to.not.throw()
+
+      closeWindow(w).then(() => { w = null })
+    })
+
     it('throws errors when the options are invalid', () => {
       expect(() => {
         dialog.showSaveDialog({ title: 300 })
@@ -51,6 +90,24 @@ describe('dialog module', () => {
   })
 
   describe('showMessageBox', () => {
+    it('should not throw for valid cases', () => {
+      // Blocks the main process and can't be run in CI
+      if (isCI) return
+
+      let w
+
+      expect(() => {
+        dialog.showMessageBox({ title: 'i am title' })
+      }).to.not.throw()
+
+      expect(() => {
+        w = new BrowserWindow()
+        dialog.showMessageBox(w, { title: 'i am title' })
+      }).to.not.throw()
+
+      closeWindow(w).then(() => { w = null })
+    })
+
     it('throws errors when the options are invalid', () => {
       expect(() => {
         dialog.showMessageBox(undefined, { type: 'not-a-valid-type' })