From ea9abe26e1c40c0d2e96007bf7d69afb49a7052a Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Fri, 25 Oct 2024 13:43:53 -0400 Subject: [PATCH] UI/Qt: Execute dialogs opened from the page asynchronously Invoking exec() entirely blocks the UI application's main thread. Qt explicitly recommends against this. In practice, it seems prevents some IPC messages from being handled by the UI until the dialog is closed by the user. Instead, use open() (which is non-blocking) and set up a signal handler to deal with the result. --- Ladybird/Qt/Tab.cpp | 54 ++++++++++++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/Ladybird/Qt/Tab.cpp b/Ladybird/Qt/Tab.cpp index fb72ffb5d98..b1231f93e0a 100644 --- a/Ladybird/Qt/Tab.cpp +++ b/Ladybird/Qt/Tab.cpp @@ -180,34 +180,46 @@ Tab::Tab(BrowserWindow* window, RefPtr parent_client, view().on_request_alert = [this](auto const& message) { m_dialog = new QMessageBox(QMessageBox::Icon::Warning, "Ladybird", qstring_from_ak_string(message), QMessageBox::StandardButton::Ok, &view()); - m_dialog->exec(); - view().alert_closed(); - m_dialog = nullptr; + QObject::connect(m_dialog, &QDialog::finished, this, [this]() { + view().alert_closed(); + m_dialog = nullptr; + }); + + m_dialog->open(); }; view().on_request_confirm = [this](auto const& message) { m_dialog = new QMessageBox(QMessageBox::Icon::Question, "Ladybird", qstring_from_ak_string(message), QMessageBox::StandardButton::Ok | QMessageBox::StandardButton::Cancel, &view()); - auto result = m_dialog->exec(); - view().confirm_closed(result == QMessageBox::StandardButton::Ok || result == QDialog::Accepted); - m_dialog = nullptr; + QObject::connect(m_dialog, &QDialog::finished, this, [this](auto result) { + view().confirm_closed(result == QMessageBox::StandardButton::Ok || result == QDialog::Accepted); + m_dialog = nullptr; + }); + + m_dialog->open(); }; view().on_request_prompt = [this](auto const& message, auto const& default_) { m_dialog = new QInputDialog(&view()); - auto& dialog = static_cast(*m_dialog); + auto& dialog = static_cast(*m_dialog); dialog.setWindowTitle("Ladybird"); dialog.setLabelText(qstring_from_ak_string(message)); dialog.setTextValue(qstring_from_ak_string(default_)); - if (dialog.exec() == QDialog::Accepted) - view().prompt_closed(ak_string_from_qstring(dialog.textValue())); - else - view().prompt_closed({}); + QObject::connect(m_dialog, &QDialog::finished, this, [this](auto result) { + if (result == QDialog::Accepted) { + auto& dialog = static_cast(*m_dialog); + view().prompt_closed(ak_string_from_qstring(dialog.textValue())); + } else { + view().prompt_closed({}); + } - m_dialog = nullptr; + m_dialog = nullptr; + }); + + m_dialog->open(); }; view().on_request_set_prompt_text = [this](auto const& message) { @@ -227,20 +239,26 @@ Tab::Tab(BrowserWindow* window, RefPtr parent_client, view().on_request_color_picker = [this](Color current_color) { m_dialog = new QColorDialog(QColor(current_color.red(), current_color.green(), current_color.blue()), &view()); - auto& dialog = static_cast(*m_dialog); + auto& dialog = static_cast(*m_dialog); dialog.setWindowTitle("Ladybird"); dialog.setOption(QColorDialog::ShowAlphaChannel, false); QObject::connect(&dialog, &QColorDialog::currentColorChanged, this, [this](QColor const& color) { view().color_picker_update(Color(color.red(), color.green(), color.blue()), Web::HTML::ColorPickerUpdateState::Update); }); - if (dialog.exec() == QDialog::Accepted) - view().color_picker_update(Color(dialog.selectedColor().red(), dialog.selectedColor().green(), dialog.selectedColor().blue()), Web::HTML::ColorPickerUpdateState::Closed); - else - view().color_picker_update({}, Web::HTML::ColorPickerUpdateState::Closed); + QObject::connect(m_dialog, &QDialog::finished, this, [this](auto result) { + if (result == QDialog::Accepted) { + auto& dialog = static_cast(*m_dialog); + view().color_picker_update(Color(dialog.selectedColor().red(), dialog.selectedColor().green(), dialog.selectedColor().blue()), Web::HTML::ColorPickerUpdateState::Closed); + } else { + view().color_picker_update({}, Web::HTML::ColorPickerUpdateState::Closed); + } - m_dialog = nullptr; + m_dialog = nullptr; + }); + + m_dialog->open(); }; view().on_request_file_picker = [this](auto const& accepted_file_types, auto allow_multiple_files) {