Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot create new window after reload the current empty window #11600

Closed
inlann opened this issue Aug 23, 2022 · 5 comments · Fixed by #11810
Closed

Cannot create new window after reload the current empty window #11600

inlann opened this issue Aug 23, 2022 · 5 comments · Fixed by #11810
Labels
electron issues related to the electron target help wanted issues meant to be picked up, require help

Comments

@inlann
Copy link
Contributor

inlann commented Aug 23, 2022

Bug Description:

New Window does not work after reload the current window.

BTW, you can open a new window (window 2) before reload it (window 1) and reload the window 2. And then, the menu item New Window of window 2 (that means window 2 is actived) does not work anymore, but the menu item New Window of window 1 (that means window 1 is actived) still works.

Steps to Reproduce:

  1. Run the electron example: (We can see the window 1)

  2. Reload the window: (Reload the window 1)

  3. Click the New Window in the menu but it does not work: (Cannot create a new window)

Additional Information

  • Operating System: MacOS
  • Theia Version: master branch
@vince-fugnitto vince-fugnitto added help wanted issues meant to be picked up, require help electron issues related to the electron target labels Aug 23, 2022
@kittaakos
Copy link
Contributor

kittaakos commented Sep 8, 2022

I am also having issues reloading the windows. I have noticed that the electron-main process and the TheiaElectronWindow receives the reload request, but all the isSender calls evaluate as false.

Wouldn't it be better to compare the even.sender.id with this._window.webContents.id here?

protected isSender(e: IpcMainEvent): boolean {
return BrowserWindow.fromId(e.sender.id) === this._window;
}

@colin-grant-work, could you please take a look? Thank you!

Related thread: electron/electron#16863 (comment)


Update:
Overriding the isSender method like this consistently fixes the issues I am locally having:

diff --git a/arduino-ide-extension/src/electron-main/theia/theia-electron-window.ts b/arduino-ide-extension/src/electron-main/theia/theia-electron-window.ts
index 8c5fd03b..6b2be2cd 100644
--- a/arduino-ide-extension/src/electron-main/theia/theia-electron-window.ts
+++ b/arduino-ide-extension/src/electron-main/theia/theia-electron-window.ts
@@ -64,4 +64,8 @@ export class TheiaElectronWindow extends DefaultTheiaElectronWindow {
       this.toDispose
     );
   }
+
+  protected override isSender(e: IpcMainEvent): boolean {
+    return e.sender.id === this._window.webContents.id;
+  }
 }

@msujew
Copy link
Member

msujew commented Sep 8, 2022

@kittaakos Thank you for investigating this. Do you plan on creating a PR on your own? I'll gladly review it.

@kittaakos
Copy link
Contributor

Sure. If this seems to be the proper fix, I am happy to push it back.

@msujew
Copy link
Member

msujew commented Sep 8, 2022

It seems pretty reasonable to me. I assume that ids aren't reused by Electron, so comparing between the BrowserWindow instances and the IDs should accomplish the same thing.

@kittaakos
Copy link
Contributor

I will take a look next week. Please assign the task to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
electron issues related to the electron target help wanted issues meant to be picked up, require help
Projects
None yet
4 participants