From a0e31bf1de51a8b6cc28c65debb72581f740b7e6 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Mon, 15 May 2023 11:49:44 -0400 Subject: [PATCH] Ladybird: Move the page context menu from the BrowserWindow to the Tab This will allow us to show different context menus depending on what element is clicked, much like we do for Browser on Serenity. --- Ladybird/BrowserWindow.cpp | 50 +++++++++++++------------------------ Ladybird/BrowserWindow.h | 26 +++++++++++++++++-- Ladybird/Tab.cpp | 17 +++++++++++++ Ladybird/Tab.h | 2 ++ Ladybird/WebContentView.cpp | 4 +-- 5 files changed, 62 insertions(+), 37 deletions(-) diff --git a/Ladybird/BrowserWindow.cpp b/Ladybird/BrowserWindow.cpp index 460ecfc3fb..429e3fd7dd 100644 --- a/Ladybird/BrowserWindow.cpp +++ b/Ladybird/BrowserWindow.cpp @@ -62,15 +62,15 @@ BrowserWindow::BrowserWindow(Browser::CookieJar& cookie_jar, StringView webdrive auto* edit_menu = menuBar()->addMenu("&Edit"); - auto* copy_action = new QAction("&Copy", this); - copy_action->setShortcuts(QKeySequence::keyBindings(QKeySequence::StandardKey::Copy)); - edit_menu->addAction(copy_action); - QObject::connect(copy_action, &QAction::triggered, this, &BrowserWindow::copy_selected_text); + m_copy_selection_action = make("&Copy", this); + m_copy_selection_action->setShortcuts(QKeySequence::keyBindings(QKeySequence::StandardKey::Copy)); + edit_menu->addAction(m_copy_selection_action); + QObject::connect(m_copy_selection_action, &QAction::triggered, this, &BrowserWindow::copy_selected_text); - auto* select_all_action = new QAction("Select &All", this); - select_all_action->setShortcuts(QKeySequence::keyBindings(QKeySequence::StandardKey::SelectAll)); - edit_menu->addAction(select_all_action); - QObject::connect(select_all_action, &QAction::triggered, this, &BrowserWindow::select_all); + m_select_all_action = make("Select &All", this); + m_select_all_action->setShortcuts(QKeySequence::keyBindings(QKeySequence::StandardKey::SelectAll)); + edit_menu->addAction(m_select_all_action); + QObject::connect(m_select_all_action, &QAction::triggered, this, &BrowserWindow::select_all); auto* view_menu = menuBar()->addMenu("&View"); @@ -133,11 +133,11 @@ BrowserWindow::BrowserWindow(Browser::CookieJar& cookie_jar, StringView webdrive auto* inspect_menu = menuBar()->addMenu("&Inspect"); - auto* view_source_action = new QAction("View &Source", this); - view_source_action->setIcon(QIcon(QString("%1/res/icons/16x16/filetype-html.png").arg(s_serenity_resource_root.characters()))); - view_source_action->setShortcut(QKeySequence(Qt::CTRL | Qt::Key_U)); - inspect_menu->addAction(view_source_action); - QObject::connect(view_source_action, &QAction::triggered, this, [this] { + m_view_source_action = make("View &Source", this); + m_view_source_action->setIcon(QIcon(QString("%1/res/icons/16x16/filetype-html.png").arg(s_serenity_resource_root.characters()))); + m_view_source_action->setShortcut(QKeySequence(Qt::CTRL | Qt::Key_U)); + inspect_menu->addAction(m_view_source_action); + QObject::connect(m_view_source_action, &QAction::triggered, this, [this] { if (m_current_tab) { m_current_tab->view().get_source(); } @@ -330,12 +330,8 @@ BrowserWindow::BrowserWindow(Browser::CookieJar& cookie_jar, StringView webdrive QObject::connect(m_tabs_container, &QTabWidget::tabCloseRequested, this, &BrowserWindow::close_tab); QObject::connect(close_current_tab_action, &QAction::triggered, this, &BrowserWindow::close_current_tab); - setContextMenuPolicy(Qt::CustomContextMenu); - QObject::connect(this, &QWidget::customContextMenuRequested, this, &BrowserWindow::show_context_menu); - - m_context_menu = make("Context menu", this); - auto* inspect_element_action = new QAction("&Inspect Element", this); - connect(inspect_element_action, &QAction::triggered, this, [this] { + m_inspect_dom_node_action = make("&Inspect Element", this); + connect(m_inspect_dom_node_action, &QAction::triggered, this, [this] { if (m_current_tab) m_current_tab->view().show_inspector(WebContentView::InspectorTarget::HoveredElement); }); @@ -354,15 +350,7 @@ BrowserWindow::BrowserWindow(Browser::CookieJar& cookie_jar, StringView webdrive if (m_current_tab) m_current_tab->reload(); }); - m_context_menu->addAction(m_go_back_action); - m_context_menu->addAction(m_go_forward_action); - m_context_menu->addAction(m_reload_action); - m_context_menu->addSeparator(); - m_context_menu->addAction(copy_action); - m_context_menu->addAction(select_all_action); - m_context_menu->addSeparator(); - m_context_menu->addAction(view_source_action); - m_context_menu->addAction(inspect_element_action); + m_go_back_action->setShortcuts(QKeySequence::keyBindings(QKeySequence::StandardKey::Back)); m_go_forward_action->setShortcuts(QKeySequence::keyBindings(QKeySequence::StandardKey::Forward)); m_reload_action->setShortcuts(QKeySequence::keyBindings(QKeySequence::StandardKey::Refresh)); @@ -372,11 +360,7 @@ BrowserWindow::BrowserWindow(Browser::CookieJar& cookie_jar, StringView webdrive new_tab(s_settings->new_tab_page(), Web::HTML::ActivateTab::Yes); setCentralWidget(m_tabs_container); -} - -void BrowserWindow::show_context_menu(QPoint const& point) -{ - m_context_menu->exec(mapToGlobal(point)); + setContextMenuPolicy(Qt::PreventContextMenu); } void BrowserWindow::set_current_tab(Tab* tab) diff --git a/Ladybird/BrowserWindow.h b/Ladybird/BrowserWindow.h index 1cc0e2c63b..0724addbe1 100644 --- a/Ladybird/BrowserWindow.h +++ b/Ladybird/BrowserWindow.h @@ -47,6 +47,26 @@ public: return *m_reload_action; } + QAction& copy_selection_action() + { + return *m_copy_selection_action; + } + + QAction& select_all_action() + { + return *m_select_all_action; + } + + QAction& view_source_action() + { + return *m_view_source_action; + } + + QAction& inspect_dom_node_action() + { + return *m_inspect_dom_node_action; + } + public slots: void tab_title_changed(int index, QString const&); void tab_favicon_changed(int index, QIcon icon); @@ -64,7 +84,6 @@ public slots: void reset_zoom(); void select_all(); void copy_selected_text(); - void show_context_menu(QPoint const&); protected: bool eventFilter(QObject* obj, QEvent* event) override; @@ -83,10 +102,13 @@ private: Tab* m_current_tab { nullptr }; QMenu* m_zoom_menu { nullptr }; - OwnPtr m_context_menu {}; OwnPtr m_go_back_action {}; OwnPtr m_go_forward_action {}; OwnPtr m_reload_action {}; + OwnPtr m_copy_selection_action {}; + OwnPtr m_select_all_action {}; + OwnPtr m_view_source_action {}; + OwnPtr m_inspect_dom_node_action {}; Browser::CookieJar& m_cookie_jar; diff --git a/Ladybird/Tab.cpp b/Ladybird/Tab.cpp index 3735601ecc..19d13b8e97 100644 --- a/Ladybird/Tab.cpp +++ b/Ladybird/Tab.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -177,6 +178,22 @@ Tab::Tab(BrowserWindow* window, StringView webdriver_content_ipc_path, WebView:: m_window->showFullScreen(); return Gfx::IntRect { m_window->x(), m_window->y(), m_window->width(), m_window->height() }; }); + + m_page_context_menu = make("Context menu", this); + m_page_context_menu->addAction(&m_window->go_back_action()); + m_page_context_menu->addAction(&m_window->go_forward_action()); + m_page_context_menu->addAction(&m_window->reload_action()); + m_page_context_menu->addSeparator(); + m_page_context_menu->addAction(&m_window->copy_selection_action()); + m_page_context_menu->addAction(&m_window->select_all_action()); + m_page_context_menu->addSeparator(); + m_page_context_menu->addAction(&m_window->view_source_action()); + m_page_context_menu->addAction(&m_window->inspect_dom_node_action()); + + view().on_context_menu_request = [this](auto widget_position) { + auto screen_position = mapToGlobal(QPoint { widget_position.x(), widget_position.y() }); + m_page_context_menu->exec(screen_position); + }; } void Tab::update_reset_zoom_button() diff --git a/Ladybird/Tab.h b/Ladybird/Tab.h index 3646a6d764..0cf50b75d6 100644 --- a/Ladybird/Tab.h +++ b/Ladybird/Tab.h @@ -67,6 +67,8 @@ private: QString m_title; QLabel* m_hover_label { nullptr }; + OwnPtr m_page_context_menu; + int tab_index(); bool m_is_history_navigation { false }; diff --git a/Ladybird/WebContentView.cpp b/Ladybird/WebContentView.cpp index d07e313421..82e63faecb 100644 --- a/Ladybird/WebContentView.cpp +++ b/Ladybird/WebContentView.cpp @@ -866,8 +866,8 @@ void WebContentView::notify_server_did_request_refresh(Badge) void WebContentView::notify_server_did_request_context_menu(Badge, Gfx::IntPoint content_position) { - // FIXME - (void)content_position; + if (on_context_menu_request) + on_context_menu_request(to_widget(content_position)); } void WebContentView::notify_server_did_request_link_context_menu(Badge, Gfx::IntPoint content_position, AK::URL const& url, DeprecatedString const&, unsigned)