refactor: use WidgetDelegate's title property (#46849)

* refactor: use WidgetDelegate::SetTitle()

* Make NativeWindow::SetTitle() and NativeWindow::GetTitle() non-virtual.
  Use WidgetDelegate for their implementation.

* Add NativeWindow::OnTitleChanged(), a new protected virtual method to update
  subclasses (e.g. NativeWindowMac needs to redraw the button proxy).

* In NativeWindowMac, replace SetTitle() and GetTitle() with OnTitleChanged().

* In NativeWindowViews, replace SetTitle() and GetTitle() with OnTitleChanged().

* test: enable BrowserWindow.title tests on Linux

* test: add a test to confirm win.title changes when document.title is set in the renderer
This commit is contained in:
Charles Kerr 2025-04-30 10:22:27 -05:00 committed by GitHub
commit 25d77fd1ce
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 60 additions and 59 deletions

View file

@ -804,6 +804,18 @@ std::u16string NativeWindow::GetAccessibleWindowTitle() const {
return accessible_title_; return accessible_title_;
} }
std::string NativeWindow::GetTitle() const {
return base::UTF16ToUTF8(WidgetDelegate::GetWindowTitle());
}
void NativeWindow::SetTitle(const std::string_view title) {
if (title == GetTitle())
return;
WidgetDelegate::SetTitle(base::UTF8ToUTF16(title));
OnTitleChanged();
}
void NativeWindow::SetAccessibleTitle(const std::string& title) { void NativeWindow::SetAccessibleTitle(const std::string& title) {
accessible_title_ = base::UTF8ToUTF16(title); accessible_title_ = base::UTF8ToUTF16(title);
} }

View file

@ -148,8 +148,6 @@ class NativeWindow : public base::SupportsUserData,
virtual ui::ZOrderLevel GetZOrderLevel() const = 0; virtual ui::ZOrderLevel GetZOrderLevel() const = 0;
virtual void Center() = 0; virtual void Center() = 0;
virtual void Invalidate() = 0; virtual void Invalidate() = 0;
virtual void SetTitle(const std::string& title) = 0;
virtual std::string GetTitle() const = 0;
#if BUILDFLAG(IS_MAC) #if BUILDFLAG(IS_MAC)
virtual std::string GetAlwaysOnTopLevel() const = 0; virtual std::string GetAlwaysOnTopLevel() const = 0;
virtual void SetActive(bool is_key) = 0; virtual void SetActive(bool is_key) = 0;
@ -160,6 +158,9 @@ class NativeWindow : public base::SupportsUserData,
virtual void DetachChildren() = 0; virtual void DetachChildren() = 0;
#endif #endif
void SetTitle(std::string_view title);
[[nodiscard]] std::string GetTitle() const;
// Ability to augment the window title for the screen readers. // Ability to augment the window title for the screen readers.
void SetAccessibleTitle(const std::string& title); void SetAccessibleTitle(const std::string& title);
std::string GetAccessibleTitle(); std::string GetAccessibleTitle();
@ -437,6 +438,8 @@ class NativeWindow : public base::SupportsUserData,
NativeWindow(const gin_helper::Dictionary& options, NativeWindow* parent); NativeWindow(const gin_helper::Dictionary& options, NativeWindow* parent);
virtual void OnTitleChanged() {}
// views::WidgetDelegate: // views::WidgetDelegate:
views::Widget* GetWidget() override; views::Widget* GetWidget() override;
const views::Widget* GetWidget() const override; const views::Widget* GetWidget() const override;

View file

@ -37,6 +37,7 @@ class NativeWindowMac : public NativeWindow,
~NativeWindowMac() override; ~NativeWindowMac() override;
// NativeWindow: // NativeWindow:
void OnTitleChanged() override;
void SetContentView(views::View* view) override; void SetContentView(views::View* view) override;
void Close() override; void Close() override;
void CloseImmediately() override; void CloseImmediately() override;
@ -85,8 +86,6 @@ class NativeWindowMac : public NativeWindow,
ui::ZOrderLevel GetZOrderLevel() const override; ui::ZOrderLevel GetZOrderLevel() const override;
void Center() override; void Center() override;
void Invalidate() override; void Invalidate() override;
void SetTitle(const std::string& title) override;
std::string GetTitle() const override;
void FlashFrame(bool flash) override; void FlashFrame(bool flash) override;
void SetSkipTaskbar(bool skip) override; void SetSkipTaskbar(bool skip) override;
void SetExcludedFromShownWindowsMenu(bool excluded) override; void SetExcludedFromShownWindowsMenu(bool excluded) override;

View file

@ -938,16 +938,13 @@ void NativeWindowMac::Invalidate() {
[[window_ contentView] setNeedsDisplay:YES]; [[window_ contentView] setNeedsDisplay:YES];
} }
void NativeWindowMac::SetTitle(const std::string& title) { void NativeWindowMac::OnTitleChanged() {
[window_ setTitle:base::SysUTF8ToNSString(title)]; [window_ setTitle:base::SysUTF8ToNSString(GetTitle())];
if (buttons_proxy_) if (buttons_proxy_)
[buttons_proxy_ redraw]; [buttons_proxy_ redraw];
} }
std::string NativeWindowMac::GetTitle() const {
return base::SysNSStringToUTF8([window_ title]);
}
void NativeWindowMac::FlashFrame(bool flash) { void NativeWindowMac::FlashFrame(bool flash) {
if (flash) { if (flash) {
attention_request_id_ = [NSApp requestUserAttention:NSCriticalRequest]; attention_request_id_ = [NSApp requestUserAttention:NSCriticalRequest];

View file

@ -198,7 +198,8 @@ class NativeWindowClientView : public views::ClientView {
NativeWindowViews::NativeWindowViews(const gin_helper::Dictionary& options, NativeWindowViews::NativeWindowViews(const gin_helper::Dictionary& options,
NativeWindow* parent) NativeWindow* parent)
: NativeWindow(options, parent) { : NativeWindow(options, parent) {
options.Get(options::kTitle, &title_); if (std::string val; options.Get(options::kTitle, &val))
SetTitle(val);
bool menu_bar_autohide; bool menu_bar_autohide;
if (options.Get(options::kAutoHideMenuBar, &menu_bar_autohide)) if (options.Get(options::kAutoHideMenuBar, &menu_bar_autohide))
@ -1160,15 +1161,6 @@ void NativeWindowViews::Invalidate() {
widget()->SchedulePaintInRect(gfx::Rect(GetBounds().size())); widget()->SchedulePaintInRect(gfx::Rect(GetBounds().size()));
} }
void NativeWindowViews::SetTitle(const std::string& title) {
title_ = title;
widget()->UpdateWindowTitle();
}
std::string NativeWindowViews::GetTitle() const {
return title_;
}
void NativeWindowViews::FlashFrame(bool flash) { void NativeWindowViews::FlashFrame(bool flash) {
#if BUILDFLAG(IS_WIN) #if BUILDFLAG(IS_WIN)
// The Chromium's implementation has a bug stopping flash. // The Chromium's implementation has a bug stopping flash.
@ -1750,10 +1742,6 @@ bool NativeWindowViews::CanMinimize() const {
#endif #endif
} }
std::u16string NativeWindowViews::GetWindowTitle() const {
return base::UTF8ToUTF16(title_);
}
views::View* NativeWindowViews::GetContentsView() { views::View* NativeWindowViews::GetContentsView() {
return root_view_.GetMainView(); return root_view_.GetMainView();
} }

View file

@ -98,8 +98,6 @@ class NativeWindowViews : public NativeWindow,
ui::ZOrderLevel GetZOrderLevel() const override; ui::ZOrderLevel GetZOrderLevel() const override;
void Center() override; void Center() override;
void Invalidate() override; void Invalidate() override;
void SetTitle(const std::string& title) override;
std::string GetTitle() const override;
void FlashFrame(bool flash) override; void FlashFrame(bool flash) override;
void SetSkipTaskbar(bool skip) override; void SetSkipTaskbar(bool skip) override;
void SetExcludedFromShownWindowsMenu(bool excluded) override {} void SetExcludedFromShownWindowsMenu(bool excluded) override {}
@ -191,7 +189,6 @@ class NativeWindowViews : public NativeWindow,
views::View* GetInitiallyFocusedView() override; views::View* GetInitiallyFocusedView() override;
bool CanMaximize() const override; bool CanMaximize() const override;
bool CanMinimize() const override; bool CanMinimize() const override;
std::u16string GetWindowTitle() const override;
views::View* GetContentsView() override; views::View* GetContentsView() override;
bool ShouldDescendIntoChildForEventHandling( bool ShouldDescendIntoChildForEventHandling(
gfx::NativeView child, gfx::NativeView child,
@ -327,7 +324,6 @@ class NativeWindowViews : public NativeWindow,
bool maximizable_ = true; bool maximizable_ = true;
bool minimizable_ = true; bool minimizable_ = true;
bool fullscreenable_ = true; bool fullscreenable_ = true;
std::string title_;
gfx::Size widget_size_; gfx::Size widget_size_;
double opacity_ = 1.0; double opacity_ = 1.0;
bool widget_destroyed_ = false; bool widget_destroyed_ = false;

View file

@ -3888,8 +3888,14 @@ describe('BrowserWindow module', () => {
}); });
it('works for window events', async () => { it('works for window events', async () => {
const pageTitleUpdated = once(w, 'page-title-updated'); const pageTitleUpdated = once(w, 'page-title-updated');
w.loadURL('data:text/html,<script>document.title = \'changed\'</script>'); const newTitle = 'changed';
w.loadURL(`data:text/html,<script>document.title = '${newTitle}'</script>`);
await pageTitleUpdated; await pageTitleUpdated;
// w.title should update after 'page-title-updated'.
// It happens right *after* the event fires though,
// so we have to waitUntil it changes
waitUntil(() => w.title === newTitle);
}); });
it('works for stop events', async () => { it('works for stop events', async () => {
@ -5428,6 +5434,36 @@ describe('BrowserWindow module', () => {
}); });
}); });
}); });
describe('native window title', () => {
describe('with properties', () => {
it('can be set with title constructor option', () => {
const w = new BrowserWindow({ show: false, title: 'mYtItLe' });
expect(w.title).to.eql('mYtItLe');
});
it('can be changed', () => {
const w = new BrowserWindow({ show: false });
expect(w.title).to.eql('Electron Test Main');
w.title = 'NEW TITLE';
expect(w.title).to.eql('NEW TITLE');
});
});
describe('with functions', () => {
it('can be set with minimizable constructor option', () => {
const w = new BrowserWindow({ show: false, title: 'mYtItLe' });
expect(w.getTitle()).to.eql('mYtItLe');
});
it('can be changed', () => {
const w = new BrowserWindow({ show: false });
expect(w.getTitle()).to.eql('Electron Test Main');
w.setTitle('NEW TITLE');
expect(w.getTitle()).to.eql('NEW TITLE');
});
});
});
}); });
ifdescribe(process.platform !== 'linux')('window states (excluding Linux)', () => { ifdescribe(process.platform !== 'linux')('window states (excluding Linux)', () => {
@ -5508,36 +5544,6 @@ describe('BrowserWindow module', () => {
}); });
}); });
describe('native window title', () => {
describe('with properties', () => {
it('can be set with title constructor option', () => {
const w = new BrowserWindow({ show: false, title: 'mYtItLe' });
expect(w.title).to.eql('mYtItLe');
});
it('can be changed', () => {
const w = new BrowserWindow({ show: false });
expect(w.title).to.eql('Electron Test Main');
w.title = 'NEW TITLE';
expect(w.title).to.eql('NEW TITLE');
});
});
describe('with functions', () => {
it('can be set with minimizable constructor option', () => {
const w = new BrowserWindow({ show: false, title: 'mYtItLe' });
expect(w.getTitle()).to.eql('mYtItLe');
});
it('can be changed', () => {
const w = new BrowserWindow({ show: false });
expect(w.getTitle()).to.eql('Electron Test Main');
w.setTitle('NEW TITLE');
expect(w.getTitle()).to.eql('NEW TITLE');
});
});
});
describe('minimizable state', () => { describe('minimizable state', () => {
describe('with properties', () => { describe('with properties', () => {
it('can be set with minimizable constructor option', () => { it('can be set with minimizable constructor option', () => {