New Tab doesn't work without an open window on macOS #13689
Comments
|
I believe @armaanahluwalia has identified the cause of this issue: See #8164 for more info @armaanahluwalia would you be up for trying a fix? 😄 |
bsclifton
added
0.22.x-single-webview
|
@bsclifton Absolutely would be my pleasure to work on this project. Will begin working on it tomorrow and try to send up a fix soon. |
Fix #8164 Fix #13689 This bug was caused because menu items were using a hidden window for their new tabs. We should _not_ use `electron{.remote}.BrowserWindow.getAllWindows()` or `.get{Active, Focused}Window` directly. There are two reasons that could have bad results: 1. We create BrowserWindows which are not normal tabbed renderer windows (e.g. some code that runs on Bookmark creation) 2. We have a hidden window sometimes (the Buffer Window) Instead call `app/browser/window.js`: `getAllRendererWindows` and `getActiveWindowId`. Fix for main menu actions, which can be called when there are no windows, but when searching for `BrowserWindow.` Test Plan: - Check all menu items open new tab in currently active window - Check all menu items open new tab in currently active window, when app is not focused - Check all menu items open new tab in new window if there is no visible window - Check tabs in all windows are persisted on restart when app / windows close - Check process quits in Windows when last visible window is closed
|
@armaanahluwalia Apologies, but I had to fix this as we want to get it in to the next release. I also noticed that the fix was a bit more complicated than I had originally thought since on Windows, the code is called by a different process which does not have access to the internal window API. That's because on Windows, the main menu is part of each window process, whereas on other platforms the main menu is part of the application process. It would be great if you could take a look at my PR for the fix at #13749. Thanks for looking in to this issue and finding the cause! If you would like to contribute to any other features / fixes, I'd be happy to help. |
|
@petemill No worries at all. Thanks for taking this on. TBH a bit busy at the moment with job interviews. Anyway, I'm not sure I'd be able to contribute productively to reviewing this PR since I haven't had the time to parse the overall structure of the application code yet. I will do soon. Thanks for the shout out. |
Fix #8164 Fix #13689 This bug was caused because menu items were using a hidden window for their new tabs. We should _not_ use `electron{.remote}.BrowserWindow.getAllWindows()` or `.get{Active, Focused}Window` directly. There are two reasons that could have bad results: 1. We create BrowserWindows which are not normal tabbed renderer windows (e.g. some code that runs on Bookmark creation) 2. We have a hidden window sometimes (the Buffer Window) Instead call `app/browser/window.js`: `getAllRendererWindows` and `getActiveWindowId`. Fix for main menu actions, which can be called when there are no windows, but when searching for `BrowserWindow.` Test Plan: - Check all menu items open new tab in currently active window - Check all menu items open new tab in currently active window, when app is not focused - Check all menu items open new tab in new window if there is no visible window - Check tabs in all windows are persisted on restart when app / windows close - Check process quits in Windows when last visible window is closed
Fix #13689 This bug was caused because menu items were using a hidden window for their new tabs. We should _not_ use `electron{.remote}.BrowserWindow.getAllWindows()` or `.get{Active, Focused}Window` directly. There are two reasons that could have bad results: 1. We create BrowserWindows which are not normal tabbed renderer windows (e.g. some code that runs on Bookmark creation) 2. We have a hidden window sometimes (the Buffer Window) Instead call `app/browser/window.js`: `getAllRendererWindows` and `getActiveWindowId`. Fix for main menu actions, which can be called when there are no windows, but when searching for `BrowserWindow.` Test Plan: - Check all menu items open new tab in currently active window - Check all menu items open new tab in currently active window, when app is not focused - Check all menu items open new tab in new window if there is no visible window - Check tabs in all windows are persisted on restart when app / windows close - Check process quits in Windows when last visible window is closed
|
Multiple +1s in community |
petemill
modified the milestones:
0.22.x Release 3 (Beta channel),
0.22.x w/ Chromium 66
|
Removed macOS label - linked test plan can mostly be checked on all platforms and does include a specific Windows check to be completed. |
Test plan
See #13749
Description
If there is no open window, New Tab (and New Private Tab and New Session Tab) does not work until New Window is selected.
This was working correctly in version 0.21.x but broke in 0.22.x.
Steps to Reproduce
macOS only
Actual result:
Nothing happens.
Expected result:
A new window with the tab should open.
Reproduces how often:
Always
Brave Version
about:brave info:
Brave: 0.22.13
V8: 6.5.254.41
rev: a8cfb16
Muon: 5.1.2
OS Release: 17.5.0
Update Channel: Release
OS Architecture: x64
OS Platform: macOS
Node.js: 7.9.0
Brave Sync: v1.4.2
libchromiumcontent: 65.0.3325.181
Reproducible on current live release:
Yes, 0.22.13.
Additional Information
The text was updated successfully, but these errors were encountered: