Ladybird/Qt: Reference the correct tab when handling the audio icon

We were errantly always referring to the active tab when the audio play
state changed, and when clicking a tab's audio state button, by way of
BrowserWindow::view().

It turns out we also can't copy / rely on the tab index provided to the
signal in any asynchronous context. If the tabs are rearranged, so are
their indices. Instead, capture a pointer to the tab of interest - this
should be safe as we wouldn't be able to click a tab's audio button if
that tab no longer exists.

With this change, we can click the audio button from any tab in the Qt
chrome, and re-arrange tabs at will. The AppKit and Serenity chromes do
not have this issue.
This commit is contained in:
Timothy Flynn 2024-03-30 18:36:26 -04:00 committed by Andreas Kling
commit 3e659b10f0
Notes: sideshowbarker 2024-07-17 07:09:53 +09:00
2 changed files with 17 additions and 14 deletions

View file

@ -649,29 +649,32 @@ void BrowserWindow::tab_favicon_changed(int index, QIcon const& icon)
void BrowserWindow::tab_audio_play_state_changed(int index, Web::HTML::AudioPlayState play_state) void BrowserWindow::tab_audio_play_state_changed(int index, Web::HTML::AudioPlayState play_state)
{ {
auto* tab = verify_cast<Tab>(m_tabs_container->widget(index));
switch (play_state) { switch (play_state) {
case Web::HTML::AudioPlayState::Paused: case Web::HTML::AudioPlayState::Paused:
if (view().page_mute_state() == Web::HTML::MuteState::Unmuted) if (tab->view().page_mute_state() == Web::HTML::MuteState::Unmuted)
m_tabs_container->tabBar()->setTabButton(index, QTabBar::LeftSide, nullptr); m_tabs_container->tabBar()->setTabButton(index, QTabBar::LeftSide, nullptr);
break; break;
case Web::HTML::AudioPlayState::Playing: case Web::HTML::AudioPlayState::Playing:
auto* button = new QPushButton(icon_for_page_mute_state(), {}); auto* button = new QPushButton(icon_for_page_mute_state(*tab), {});
button->setToolTip(tool_tip_for_page_mute_state()); button->setToolTip(tool_tip_for_page_mute_state(*tab));
button->setFlat(true); button->setFlat(true);
button->resize({ 20, 20 }); button->resize({ 20, 20 });
connect(button, &QPushButton::clicked, this, [this, index]() { connect(button, &QPushButton::clicked, this, [this, tab]() {
view().toggle_page_mute_state(); tab->view().toggle_page_mute_state();
auto index = tab_index(tab);
switch (view().audio_play_state()) { switch (tab->view().audio_play_state()) {
case Web::HTML::AudioPlayState::Paused: case Web::HTML::AudioPlayState::Paused:
m_tabs_container->tabBar()->setTabButton(index, QTabBar::LeftSide, nullptr); m_tabs_container->tabBar()->setTabButton(index, QTabBar::LeftSide, nullptr);
break; break;
case Web::HTML::AudioPlayState::Playing: case Web::HTML::AudioPlayState::Playing:
auto* button = m_tabs_container->tabBar()->tabButton(index, QTabBar::LeftSide); auto* button = m_tabs_container->tabBar()->tabButton(index, QTabBar::LeftSide);
verify_cast<QPushButton>(button)->setIcon(icon_for_page_mute_state()); verify_cast<QPushButton>(button)->setIcon(icon_for_page_mute_state(*tab));
button->setToolTip(tool_tip_for_page_mute_state()); button->setToolTip(tool_tip_for_page_mute_state(*tab));
break; break;
} }
}); });
@ -681,9 +684,9 @@ void BrowserWindow::tab_audio_play_state_changed(int index, Web::HTML::AudioPlay
} }
} }
QIcon BrowserWindow::icon_for_page_mute_state() const QIcon BrowserWindow::icon_for_page_mute_state(Tab& tab) const
{ {
switch (view().page_mute_state()) { switch (tab.view().page_mute_state()) {
case Web::HTML::MuteState::Muted: case Web::HTML::MuteState::Muted:
return style()->standardIcon(QStyle::SP_MediaVolumeMuted); return style()->standardIcon(QStyle::SP_MediaVolumeMuted);
case Web::HTML::MuteState::Unmuted: case Web::HTML::MuteState::Unmuted:
@ -693,9 +696,9 @@ QIcon BrowserWindow::icon_for_page_mute_state() const
VERIFY_NOT_REACHED(); VERIFY_NOT_REACHED();
} }
QString BrowserWindow::tool_tip_for_page_mute_state() const QString BrowserWindow::tool_tip_for_page_mute_state(Tab& tab) const
{ {
switch (view().page_mute_state()) { switch (tab.view().page_mute_state()) {
case Web::HTML::MuteState::Muted: case Web::HTML::MuteState::Muted:
return "Unmute tab"; return "Unmute tab";
case Web::HTML::MuteState::Unmuted: case Web::HTML::MuteState::Unmuted:

View file

@ -128,8 +128,8 @@ private:
} }
} }
QIcon icon_for_page_mute_state() const; QIcon icon_for_page_mute_state(Tab&) const;
QString tool_tip_for_page_mute_state() const; QString tool_tip_for_page_mute_state(Tab&) const;
QScreen* m_current_screen; QScreen* m_current_screen;
double m_device_pixel_ratio { 0 }; double m_device_pixel_ratio { 0 };