Make sure we can reopen secondary windows (#13509)

- Refactored to close widget upon browser-case window close - Seems 'close' event is no longer fired in browser case. Using 'unload' Fixes #13214 Contributred on behalf of STMicroelectronics. Signed-off-by: Thomas Mäder <[email protected]>
eclipse-theia · Mar 21, 2024 · c500d0e · c500d0e
1 parent cf63b20
commit c500d0e
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 34 deletions.
diff --git a/packages/core/src/browser/window/default-secondary-window-service.ts b/packages/core/src/browser/window/default-secondary-window-service.ts
@@ -82,37 +82,14 @@ export class DefaultSecondaryWindowService implements SecondaryWindowService {
     }
 
     createSecondaryWindow(widget: ExtractableWidget, shell: ApplicationShell): Window | undefined {
-        const win = this.doCreateSecondaryWindow(widget, shell);
-        if (win) {
-            this.secondaryWindows.push(win);
-            win.addEventListener('close', () => {
-                const extIndex = this.secondaryWindows.indexOf(win);
-                if (extIndex > -1) {
-                    this.secondaryWindows.splice(extIndex, 1);
-                };
-            });
-        }
-        return win;
-    }
-
-    protected findWindow<T>(windowName: string): Window | undefined {
-        for (const w of this.secondaryWindows) {
-            if (w.name === windowName) {
-                return w;
-            }
-        }
-        return undefined;
-    }
-
-    protected doCreateSecondaryWindow(widget: ExtractableWidget, shell: ApplicationShell): Window | undefined {
-        let options;
         const [height, width, left, top] = this.findSecondaryWindowCoordinates(widget);
-        options = `popup=1,width=${width},height=${height},left=${left},top=${top}`;
+        let options = `popup=1,width=${width},height=${height},left=${left},top=${top}`;
         if (this.preferenceService.get('window.secondaryWindowAlwaysOnTop')) {
             options += ',alwaysOnTop=true';
         }
         const newWindow = window.open(DefaultSecondaryWindowService.SECONDARY_WINDOW_URL, this.nextWindowId(), options) ?? undefined;
         if (newWindow) {
+            this.secondaryWindows.push(newWindow);
             newWindow.addEventListener('DOMContentLoaded', () => {
                 newWindow.addEventListener('beforeunload', evt => {
                     const saveable = Saveable.get(widget);
@@ -124,17 +101,38 @@ export class DefaultSecondaryWindowService implements SecondaryWindowService {
                     }
                 }, { capture: true });
 
-                newWindow.addEventListener('close', () => {
+                newWindow.addEventListener('unload', () => {
                     const saveable = Saveable.get(widget);
                     shell.closeWidget(widget.id, {
                         save: !!saveable && saveable.dirty && saveable.autoSave !== 'off'
                     });
+
+                    const extIndex = this.secondaryWindows.indexOf(newWindow);
+                    if (extIndex > -1) {
+                        this.secondaryWindows.splice(extIndex, 1);
+                    };
                 });
+                this.windowCreated(newWindow, widget, shell);
             });
         }
         return newWindow;
     }
 
+    protected windowCreated(newWindow: Window, widget: ExtractableWidget, shell: ApplicationShell): void {
+        newWindow.addEventListener('unload', () => {
+            shell.closeWidget(widget.id);
+        });
+    }
+
+    protected findWindow<T>(windowName: string): Window | undefined {
+        for (const w of this.secondaryWindows) {
+            if (w.name === windowName) {
+                return w;
+            }
+        }
+        return undefined;
+    }
+
     protected findSecondaryWindowCoordinates(widget: ExtractableWidget): (number | undefined)[] {
         const clientBounds = widget.node.getBoundingClientRect();
         const preference = this.preferenceService.get('window.secondaryWindowPlacement');

diff --git a/packages/core/src/electron-browser/window/electron-secondary-window-service.ts b/packages/core/src/electron-browser/window/electron-secondary-window-service.ts
@@ -24,16 +24,12 @@ export class ElectronSecondaryWindowService extends DefaultSecondaryWindowServic
         window.electronTheiaCore.focusWindow(win.name);
     }
 
-    protected override doCreateSecondaryWindow(widget: ExtractableWidget, shell: ApplicationShell): Window | undefined {
-        const w = super.doCreateSecondaryWindow(widget, shell);
-        if (w) {
-            window.electronTheiaCore.setMenuBarVisible(false, w.name);
-            window.electronTheiaCore.setSecondaryWindowCloseRequestHandler(w.name, () => this.canClose(widget, shell));
-        }
-        return w;
+    protected override windowCreated(newWindow: Window, widget: ExtractableWidget, shell: ApplicationShell): void {
+        window.electronTheiaCore.setMenuBarVisible(false, newWindow.name);
+        window.electronTheiaCore.setSecondaryWindowCloseRequestHandler(newWindow.name, () => this.canClose(widget, shell));
     }
     private async canClose(widget: ExtractableWidget, shell: ApplicationShell): Promise<boolean> {
-        await shell.closeWidget(widget.id, undefined);
+        await shell.closeWidget(widget.id);
         return widget.isDisposed;
     }
 }