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

[electron]: Fixed the window reloading issue. #8345

Merged
merged 1 commit into from Aug 17, 2020

Conversation

kittaakos
Copy link
Contributor

Refs:

What it does

Fixes the browser window reload on Windows. @mcgordonite, could you please verify the behavior with and without this change on Windows?

How to test

Try to reset the workbench layout. It should reload the browser window.

Review checklist

Reminder for reviewers

@akosyakov akosyakov added the electron issues related to the electron target label Aug 11, 2020
@akosyakov
Copy link
Member

@vince-fugnitto @marechal-p could you check please? it sounds like an important regression after Electron 9 migration

@mcgordonite
Copy link
Contributor

Hi @kittaakos, I should have time to verify this change tomorrow.

@kittaakos
Copy link
Contributor Author

@mcgordonite, do you happen to have time to verify the Reset Workbench Layout command behavior on Windows. With the master version, the browser window never reloads after resetting the workbench layout with the electron example. With this change, it should work, hopefully 🤞 Thank you!

@mcgordonite
Copy link
Contributor

@kittaakos just tested the behaviour on Windows:

  • On master the workbench reset starts the page reload, then hangs on the loading spinner. This also happens when using the "reload window" command.
  • On this PR, with allowRendererProcessReuse false, workbench reset works correctly.

@kittaakos
Copy link
Contributor Author

just tested

Thank you for your help, @mcgordonite

@vince-fugnitto
Copy link
Member

@kittaakos unfortunately I was unable to test since I don't have a Windows machine.
I did try the pull-request on Linux and did not notice any regressions.

@akosyakov
Copy link
Member

Is it fine to approve on @mcgordonite behalf or someone else is going to try?

@kittaakos
Copy link
Contributor Author

@tsmaeder, can you please help us with the review?

Copy link
Member

@paul-marechal paul-marechal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kittaakos just tested the behaviour on Windows:

  • On master the workbench reset starts the page reload, then hangs on the loading spinner. This also happens when using the "reload window" command.
  • On this PR, with allowRendererProcessReuse false, workbench reset works correctly.

Tried on Windows too and I can confirm that this fixes the issue. According to the doc the old value was false as well, so LGTM.

@kittaakos kittaakos merged commit ed5398a into master Aug 17, 2020
@kittaakos kittaakos deleted the win32-fix-electron-window-reload branch August 17, 2020 06:58
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants