mirror of
https://github.com/electron/electron.git
synced 2026-04-10 03:01:51 -04:00
feat: enforce unique window names across BaseWindow and BrowserWindow (#47764)
* feat: save window state (#47425) * feat: save/restore window state * cleanup * remove constructor option * refactor: apply suggestions from code review Co-authored-by: Charles Kerr <charles@charleskerr.com> * refactor: forward declare prefservice * refactor: remove constructor option * refactor: save window state on move/resize instead of moved/resized * feat: resave window state after construction * test: add basic window save tests * test: add work area tests * test: asynchronous batching behavior * docs: add windowStateRestoreOptions to BaseWindowConstructorOptions * chore: move includes to main block * Update spec/api-browser-window-spec.ts Co-authored-by: David Sanders <dsanders11@ucsbalum.com> * docs: update docs/api/structures/base-window-options.md Co-authored-by: Erick Zhao <erick@hotmail.ca> * fix: preserve original bounds during window state save in special modes * feat: save kiosk state in window preferences * chore: remove ts-expect-error * test: check hasCapturableScreen before running tests * test: remove multimonitor tests * test: add missing hasCapturableScreen checks before tests * docs: add blurb on saving mechanism * feat: add debounce window of 200ms to saveWindowState * docs: remove blurb until finalized * style: convert constants from snake_case to camelCase * refactor: initialize prefs_ only if window state is configured to be saved/restored * refactor: rename window states key * refactor: store in application-level Local State instead of browser context * refactor: switch to more accurate function names * fix: add dcheck for browser_process * fix: flush window state to avoid race condition * refactor: change stateId to name * refactor: change windowStateRestoreOptions to windowStatePersistence * Update docs/api/structures/base-window-options.md Co-authored-by: David Sanders <dsanders11@ucsbalum.com> * fix: add warning when window state persistence enabled without window name * docs: lowercase capital B for consistency --------- Co-authored-by: Charles Kerr <charles@charleskerr.com> Co-authored-by: David Sanders <dsanders11@ucsbalum.com> Co-authored-by: Erick Zhao <erick@hotmail.ca> * feat: enforce unique window names across BaseWindow and BrowserWindow * docs: update docs for name property * fix: linter issue with symbol --------- Co-authored-by: Charles Kerr <charles@charleskerr.com> Co-authored-by: David Sanders <dsanders11@ucsbalum.com> Co-authored-by: Erick Zhao <erick@hotmail.ca>
This commit is contained in:
committed by
Keeley Hammond
parent
a6093b1575
commit
aaf813a0f6
@@ -42,7 +42,7 @@
|
||||
Default is `false`.
|
||||
* `hiddenInMissionControl` boolean (optional) _macOS_ - Whether window should be hidden when the user toggles into mission control.
|
||||
* `kiosk` boolean (optional) - Whether the window is in kiosk mode. Default is `false`.
|
||||
* `name` string (optional) - An identifier for the window that enables features such as state persistence.
|
||||
* `name` string (optional) - A unique identifier for the window, used to enable features such as state persistence. Each window must have a distinct name. It can only be reused after the corresponding window has been destroyed.
|
||||
* `windowStatePersistence` ([WindowStatePersistence](window-state-persistence.md) | boolean) (optional) - Configures or enables the persistence of window state (position, size, maximized state, etc.) across application restarts. Has no effect if window `name` is not provided. _Experimental_
|
||||
* `title` string (optional) - Default window title. Default is `"Electron"`. If the HTML tag `<title>` is defined in the HTML file loaded by `loadURL()`, this property will be ignored.
|
||||
* `icon` ([NativeImage](../native-image.md) | string) (optional) - The window icon. On Windows it is
|
||||
|
||||
@@ -18,6 +18,7 @@
|
||||
#include "shell/browser/api/electron_api_web_contents.h"
|
||||
#include "shell/browser/javascript_environment.h"
|
||||
#include "shell/browser/native_window.h"
|
||||
#include "shell/browser/window_list.h"
|
||||
#include "shell/common/color_util.h"
|
||||
#include "shell/common/gin_converters/callback_converter.h"
|
||||
#include "shell/common/gin_converters/file_path_converter.h"
|
||||
@@ -1148,9 +1149,38 @@ gin_helper::WrappableBase* BaseWindow::New(gin_helper::Arguments* args) {
|
||||
auto options = gin_helper::Dictionary::CreateEmpty(args->isolate());
|
||||
args->GetNext(&options);
|
||||
|
||||
std::string error_message;
|
||||
if (!IsWindowNameValid(options, &error_message)) {
|
||||
// Window name is already in use throw an error and do not create the window
|
||||
args->ThrowError(error_message);
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
return new BaseWindow(args, options);
|
||||
}
|
||||
|
||||
// static
|
||||
bool BaseWindow::IsWindowNameValid(const gin_helper::Dictionary& options,
|
||||
std::string* error_message) {
|
||||
std::string window_name;
|
||||
if (options.Get(options::kName, &window_name) && !window_name.empty()) {
|
||||
// Check if window name is already in use by another window
|
||||
// Window names must be unique for state persistence to work correctly
|
||||
const auto& windows = electron::WindowList::GetWindows();
|
||||
bool name_in_use = std::any_of(windows.begin(), windows.end(),
|
||||
[&window_name](const auto* const window) {
|
||||
return window->GetName() == window_name;
|
||||
});
|
||||
|
||||
if (name_in_use) {
|
||||
*error_message = "Window name '" + window_name +
|
||||
"' is already in use. Window names must be unique.";
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
// static
|
||||
void BaseWindow::BuildPrototype(v8::Isolate* isolate,
|
||||
v8::Local<v8::FunctionTemplate> prototype) {
|
||||
|
||||
@@ -42,6 +42,9 @@ class BaseWindow : public gin_helper::TrackableObject<BaseWindow>,
|
||||
static void BuildPrototype(v8::Isolate* isolate,
|
||||
v8::Local<v8::FunctionTemplate> prototype);
|
||||
|
||||
static bool IsWindowNameValid(const gin_helper::Dictionary& options,
|
||||
std::string* error_message);
|
||||
|
||||
const NativeWindow* window() const { return window_.get(); }
|
||||
NativeWindow* window() { return window_.get(); }
|
||||
|
||||
|
||||
@@ -307,6 +307,13 @@ gin_helper::WrappableBase* BrowserWindow::New(gin_helper::ErrorThrower thrower,
|
||||
options = gin::Dictionary::CreateEmpty(args->isolate());
|
||||
}
|
||||
|
||||
std::string error_message;
|
||||
if (!IsWindowNameValid(options, &error_message)) {
|
||||
// Window name is already in use throw an error and do not create the window
|
||||
thrower.ThrowError(error_message);
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
return new BrowserWindow(args, options);
|
||||
}
|
||||
|
||||
|
||||
@@ -790,10 +790,14 @@ void NativeWindow::SetAccessibleTitle(const std::string& title) {
|
||||
WidgetDelegate::SetAccessibleTitle(base::UTF8ToUTF16(title));
|
||||
}
|
||||
|
||||
std::string NativeWindow::GetAccessibleTitle() {
|
||||
std::string NativeWindow::GetAccessibleTitle() const {
|
||||
return base::UTF16ToUTF8(GetAccessibleWindowTitle());
|
||||
}
|
||||
|
||||
std::string NativeWindow::GetName() const {
|
||||
return window_name_;
|
||||
}
|
||||
|
||||
void NativeWindow::HandlePendingFullscreenTransitions() {
|
||||
if (pending_transitions_.empty()) {
|
||||
set_fullscreen_transition_type(FullScreenTransitionType::kNone);
|
||||
|
||||
@@ -171,9 +171,11 @@ class NativeWindow : public base::SupportsUserData,
|
||||
void SetTitle(std::string_view title);
|
||||
[[nodiscard]] std::string GetTitle() const;
|
||||
|
||||
[[nodiscard]] std::string GetName() const;
|
||||
|
||||
// Ability to augment the window title for the screen readers.
|
||||
void SetAccessibleTitle(const std::string& title);
|
||||
std::string GetAccessibleTitle();
|
||||
[[nodiscard]] std::string GetAccessibleTitle() const;
|
||||
|
||||
virtual void FlashFrame(bool flash) = 0;
|
||||
virtual void SetSkipTaskbar(bool skip) = 0;
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { app, BrowserWindow, BrowserView, dialog, ipcMain, OnBeforeSendHeadersListenerDetails, net, protocol, screen, webContents, webFrameMain, session, WebContents, WebFrameMain } from 'electron/main';
|
||||
import { app, BrowserWindow, BaseWindow, BrowserView, dialog, ipcMain, OnBeforeSendHeadersListenerDetails, net, protocol, screen, webContents, webFrameMain, session, WebContents, WebFrameMain } from 'electron/main';
|
||||
|
||||
import { expect } from 'chai';
|
||||
|
||||
@@ -57,6 +57,8 @@ describe('BrowserWindow module', () => {
|
||||
});
|
||||
|
||||
describe('BrowserWindow constructor', () => {
|
||||
afterEach(closeAllWindows);
|
||||
|
||||
it('allows passing void 0 as the webContents', async () => {
|
||||
expect(() => {
|
||||
const w = new BrowserWindow({
|
||||
@@ -83,6 +85,38 @@ describe('BrowserWindow module', () => {
|
||||
w.destroy();
|
||||
}).not.to.throw();
|
||||
});
|
||||
|
||||
it('throws error when creating windows with duplicate names', () => {
|
||||
const w1 = new BrowserWindow({ show: false, name: 'duplicate-name' });
|
||||
|
||||
expect(() => {
|
||||
// eslint-disable-next-line no-new
|
||||
new BrowserWindow({ show: false, name: 'duplicate-name' });
|
||||
}).to.throw("Window name 'duplicate-name' is already in use. Window names must be unique.");
|
||||
|
||||
w1.destroy();
|
||||
});
|
||||
|
||||
it('prevents BaseWindow and BrowserWindow from using same name', () => {
|
||||
const base = new BaseWindow({ show: false, name: 'shared-name' });
|
||||
|
||||
expect(() => {
|
||||
// eslint-disable-next-line no-new
|
||||
new BrowserWindow({ show: false, name: 'shared-name' });
|
||||
}).to.throw("Window name 'shared-name' is already in use. Window names must be unique.");
|
||||
|
||||
base.destroy();
|
||||
});
|
||||
|
||||
it('allows reusing name after window is destroyed', () => {
|
||||
const w1 = new BrowserWindow({ show: false, name: 'reusable-name' });
|
||||
w1.destroy();
|
||||
|
||||
expect(() => {
|
||||
const w2 = new BrowserWindow({ show: false, name: 'reusable-name' });
|
||||
w2.destroy();
|
||||
}).not.to.throw();
|
||||
});
|
||||
});
|
||||
|
||||
describe('garbage collection', () => {
|
||||
|
||||
Reference in New Issue
Block a user