From 972c19b2922d4954ba721652f497c82214f15123 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Wed, 27 May 2026 18:12:10 +0200 Subject: [PATCH] UI/Qt: Remove icons from menus Let application actions opt out of icon creation when created for menus. Use that path for browser menus, tab context menus, and WebView-backed popup menus. Keep icon creation for toolbar and bookmarks bar actions, where the same helper still provides visible button icons. Remove the icon setup that is now only used by menu actions, and shrink popup menu item padding now that there is no icon column. --- UI/Qt/BrowserWindow.cpp | 15 ++-- UI/Qt/BrowserWindow.h | 1 - UI/Qt/ChromeStyle.cpp | 2 +- UI/Qt/Menu.cpp | 153 ++++++++-------------------------------- UI/Qt/Menu.h | 7 +- UI/Qt/Tab.cpp | 3 +- 6 files changed, 43 insertions(+), 138 deletions(-) diff --git a/UI/Qt/BrowserWindow.cpp b/UI/Qt/BrowserWindow.cpp index 45bbf581f1..e499c1347d 100644 --- a/UI/Qt/BrowserWindow.cpp +++ b/UI/Qt/BrowserWindow.cpp @@ -254,13 +254,11 @@ BrowserWindow::BrowserWindow(Vector const& initial_urls, IsPopupWindow update_reopen_recently_closed_action(); auto* close_current_tab_action = new QAction("&Close Current Tab", this); - close_current_tab_action->setIcon(load_icon_from_uri("resource://icons/16x16/close-tab.png"sv)); close_current_tab_action->setShortcuts(QKeySequence::keyBindings(QKeySequence::StandardKey::Close)); m_hamburger_menu->addAction(close_current_tab_action); file_menu->addAction(close_current_tab_action); auto* open_file_action = new QAction("&Open File...", this); - open_file_action->setIcon(load_icon_from_uri("resource://icons/16x16/filetype-folder-open.png"sv)); open_file_action->setShortcut(QKeySequence(QKeySequence::StandardKey::Open)); m_hamburger_menu->addAction(open_file_action); file_menu->addAction(open_file_action); @@ -270,14 +268,13 @@ BrowserWindow::BrowserWindow(Vector const& initial_urls, IsPopupWindow auto* edit_menu = m_hamburger_menu->addMenu("&Edit"); menuBar()->addMenu(edit_menu); - edit_menu->addAction(create_application_action(*this, Application::the().cut_selection_action())); - edit_menu->addAction(create_application_action(*this, Application::the().copy_selection_action())); - edit_menu->addAction(create_application_action(*this, Application::the().paste_action())); - edit_menu->addAction(create_application_action(*this, Application::the().select_all_action())); + edit_menu->addAction(create_application_action(*this, Application::the().cut_selection_action(), IncludeActionIcon::No)); + edit_menu->addAction(create_application_action(*this, Application::the().copy_selection_action(), IncludeActionIcon::No)); + edit_menu->addAction(create_application_action(*this, Application::the().paste_action(), IncludeActionIcon::No)); + edit_menu->addAction(create_application_action(*this, Application::the().select_all_action(), IncludeActionIcon::No)); edit_menu->addSeparator(); m_find_in_page_action = new QAction("&Find in Page...", this); - m_find_in_page_action->setIcon(load_icon_from_uri("resource://icons/16x16/find.png"sv)); m_find_in_page_action->setShortcuts(QKeySequence::keyBindings(QKeySequence::StandardKey::Find)); auto find_previous_shortcuts = QKeySequence::keyBindings(QKeySequence::StandardKey::FindPrevious); @@ -298,7 +295,7 @@ BrowserWindow::BrowserWindow(Vector const& initial_urls, IsPopupWindow QObject::connect(m_find_in_page_action, &QAction::triggered, this, &BrowserWindow::show_find_in_page); edit_menu->addSeparator(); - edit_menu->addAction(create_application_action(*edit_menu, Application::the().open_settings_page_action())); + edit_menu->addAction(create_application_action(*edit_menu, Application::the().open_settings_page_action(), IncludeActionIcon::No)); auto* view_menu = m_hamburger_menu->addMenu("&View"); menuBar()->addMenu(view_menu); @@ -346,7 +343,7 @@ BrowserWindow::BrowserWindow(Vector const& initial_urls, IsPopupWindow auto* help_menu = m_hamburger_menu->addMenu("&Help"); menuBar()->addMenu(help_menu); - help_menu->addAction(create_application_action(*help_menu, Application::the().open_about_page_action())); + help_menu->addAction(create_application_action(*help_menu, Application::the().open_about_page_action(), IncludeActionIcon::No)); m_hamburger_menu->addSeparator(); file_menu->addSeparator(); diff --git a/UI/Qt/BrowserWindow.h b/UI/Qt/BrowserWindow.h index 89932b3df3..0383ba9c16 100644 --- a/UI/Qt/BrowserWindow.h +++ b/UI/Qt/BrowserWindow.h @@ -99,7 +99,6 @@ public: QMenu& hamburger_menu() const { return *m_hamburger_menu; } - QAction& new_tab_action() const { return *m_new_tab_action; } QAction& new_window_action() const { return *m_new_window_action; } QAction& find_action() const { return *m_find_in_page_action; } diff --git a/UI/Qt/ChromeStyle.cpp b/UI/Qt/ChromeStyle.cpp index 0b2acde9ca..bf8a7c5cb6 100644 --- a/UI/Qt/ChromeStyle.cpp +++ b/UI/Qt/ChromeStyle.cpp @@ -265,7 +265,7 @@ QMenu::item {{ background: transparent; border-radius: 5px; min-height: 20px; - padding: 5px 28px; + padding: 5px 14px; }} QMenu::item:selected {{ diff --git a/UI/Qt/Menu.cpp b/UI/Qt/Menu.cpp index 7c9062b4a2..06a432779d 100644 --- a/UI/Qt/Menu.cpp +++ b/UI/Qt/Menu.cpp @@ -19,9 +19,9 @@ namespace Ladybird { class ActionObserver final : public WebView::Action::Observer { public: - static NonnullOwnPtr create(WebView::Action& action, QAction& qaction) + static NonnullOwnPtr create(WebView::Action& action, QAction& qaction, IncludeActionIcon include_action_icon) { - return adopt_own(*new ActionObserver(action, qaction)); + return adopt_own(*new ActionObserver(action, qaction, include_action_icon)); } virtual void on_text_changed(WebView::Action& action) override @@ -50,6 +50,9 @@ public: virtual void on_engaged_state_changed(WebView::Action& action) override { + if (m_include_action_icon == IncludeActionIcon::No) + return; + if (!m_action) return; @@ -73,8 +76,9 @@ public: } private: - ActionObserver(WebView::Action& action, QAction& qaction) + ActionObserver(WebView::Action& action, QAction& qaction, IncludeActionIcon include_action_icon) : m_action(&qaction) + , m_include_action_icon(include_action_icon) { QObject::connect(m_action, &QAction::triggered, [weak_action = action.make_weak_ptr()](bool checked) { if (auto action = weak_action.strong_ref()) { @@ -90,6 +94,7 @@ private: } QPointer m_action; + IncludeActionIcon m_include_action_icon { IncludeActionIcon::Yes }; }; template @@ -114,70 +119,66 @@ static QIcon icon_from_base64_png(StringView favicon_base64_png) return pixmap.scaled(MENU_ICON_SIZE, MENU_ICON_SIZE, Qt::KeepAspectRatio, Qt::SmoothTransformation); } -static void initialize_native_control(WebView::Action& action, QAction& qaction, QPalette const& palette) +static void initialize_native_control(WebView::Action& action, QAction& qaction, QPalette const& palette, IncludeActionIcon include_action_icon) { switch (action.id()) { case WebView::ActionID::NavigateBack: - qaction.setIcon(create_chrome_icon(ChromeIcon::Back, palette)); + if (include_action_icon == IncludeActionIcon::Yes) + qaction.setIcon(create_chrome_icon(ChromeIcon::Back, palette)); qaction.setShortcut(QKeySequence::StandardKey::Back); break; case WebView::ActionID::NavigateForward: - qaction.setIcon(create_chrome_icon(ChromeIcon::Forward, palette)); + if (include_action_icon == IncludeActionIcon::Yes) + qaction.setIcon(create_chrome_icon(ChromeIcon::Forward, palette)); qaction.setShortcut(QKeySequence::StandardKey::Forward); break; case WebView::ActionID::Reload: - qaction.setIcon(create_chrome_icon(ChromeIcon::Reload, palette)); + if (include_action_icon == IncludeActionIcon::Yes) + qaction.setIcon(create_chrome_icon(ChromeIcon::Reload, palette)); qaction.setShortcuts({ QKeySequence(Qt::CTRL | Qt::Key_R), QKeySequence(Qt::Key_F5) }); break; case WebView::ActionID::CopySelection: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/edit-copy.png"sv)); qaction.setShortcut(QKeySequence::StandardKey::Copy); break; case WebView::ActionID::CutSelection: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/edit-cut.png"sv)); qaction.setShortcut(QKeySequence::StandardKey::Cut); break; case WebView::ActionID::Paste: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/paste.png"sv)); qaction.setShortcut(QKeySequence::StandardKey::Paste); break; case WebView::ActionID::SelectAll: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/select-all.png"sv)); qaction.setShortcut(QKeySequence::StandardKey::SelectAll); break; - case WebView::ActionID::SearchSelectedText: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/find.png"sv)); - break; - case WebView::ActionID::ToggleBookmark: - qaction.setIcon(create_chrome_icon(action.engaged() ? ChromeIcon::StarFilled : ChromeIcon::Star, palette)); + if (include_action_icon == IncludeActionIcon::Yes) + qaction.setIcon(create_chrome_icon(action.engaged() ? ChromeIcon::StarFilled : ChromeIcon::Star, palette)); qaction.setShortcut(QKeySequence(Qt::CTRL | Qt::Key_D)); break; case WebView::ActionID::ToggleBookmarkViaToolbar: - qaction.setIcon(create_chrome_icon(action.engaged() ? ChromeIcon::StarFilled : ChromeIcon::Star, palette)); + if (include_action_icon == IncludeActionIcon::Yes) + qaction.setIcon(create_chrome_icon(action.engaged() ? ChromeIcon::StarFilled : ChromeIcon::Star, palette)); break; case WebView::ActionID::ToggleBookmarksBar: qaction.setShortcut(QKeySequence(Qt::CTRL | Qt::SHIFT | Qt::Key_B)); break; case WebView::ActionID::BookmarkItem: - if (auto icon = action.base64_png_icon(); icon.has_value()) - qaction.setIcon(icon_from_base64_png(*icon)); - else - qaction.setIcon(create_tvg_icon_with_theme_colors("globe", palette)); + if (include_action_icon == IncludeActionIcon::Yes) { + if (auto icon = action.base64_png_icon(); icon.has_value()) + qaction.setIcon(icon_from_base64_png(*icon)); + else + qaction.setIcon(create_tvg_icon_with_theme_colors("globe", palette)); + } break; case WebView::ActionID::OpenProcessesPage: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/app-system-monitor.png"sv)); qaction.setShortcut(QKeySequence(Qt::CTRL | Qt::SHIFT | Qt::Key_M)); break; case WebView::ActionID::OpenSettingsPage: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/settings.png"sv)); qaction.setShortcut(QKeySequence::StandardKey::Preferences); break; case WebView::ActionID::ToggleDevTools: - qaction.setIcon(load_icon_from_uri("resource://icons/browser/dom-tree.png"sv)); qaction.setShortcuts({ QKeySequence(Qt::CTRL | Qt::SHIFT | Qt::Key_I), QKeySequence(Qt::CTRL | Qt::SHIFT | Qt::Key_C), @@ -185,62 +186,10 @@ static void initialize_native_control(WebView::Action& action, QAction& qaction, }); break; case WebView::ActionID::ViewSource: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/filetype-html.png"sv)); qaction.setShortcut(QKeySequence(Qt::CTRL | Qt::Key_U)); break; - case WebView::ActionID::TakeVisibleScreenshot: - case WebView::ActionID::TakeFullScreenshot: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/filetype-image.png"sv)); - break; - - case WebView::ActionID::OpenInNewTab: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/new-tab.png"sv)); - break; - case WebView::ActionID::OpenInNewWindow: - // FIXME: should be a separate icon for new window. - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/new-tab.png"sv)); - break; - case WebView::ActionID::CopyURL: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/edit-copy.png"sv)); - break; - - case WebView::ActionID::OpenImage: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/filetype-image.png"sv)); - break; - case WebView::ActionID::SaveImage: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/download.png"sv)); - break; - case WebView::ActionID::CopyImage: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/edit-copy.png"sv)); - break; - - case WebView::ActionID::OpenAudio: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/filetype-sound.png"sv)); - break; - case WebView::ActionID::OpenVideo: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/filetype-video.png"sv)); - break; - case WebView::ActionID::PlayMedia: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/play.png"sv)); - break; - case WebView::ActionID::PauseMedia: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/pause.png"sv)); - break; - case WebView::ActionID::MuteMedia: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/audio-volume-muted.png"sv)); - break; - case WebView::ActionID::UnmuteMedia: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/audio-volume-high.png"sv)); - break; - case WebView::ActionID::EnterFullscreen: - case WebView::ActionID::ExitFullscreen: // FIXME: Create a separate icon for exiting fullscreen. - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/fullscreen.png"sv)); - break; - case WebView::ActionID::ZoomIn: { - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/zoom-in.png"sv)); - auto zoom_in_shortcuts = QKeySequence::keyBindings(QKeySequence::StandardKey::ZoomIn); auto secondary_zoom_in_shortcut = QKeySequence(Qt::CTRL | Qt::Key_Equal); @@ -251,48 +200,12 @@ static void initialize_native_control(WebView::Action& action, QAction& qaction, break; } case WebView::ActionID::ZoomOut: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/zoom-out.png"sv)); qaction.setShortcut(QKeySequence::StandardKey::ZoomOut); break; case WebView::ActionID::ResetZoom: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/zoom-reset.png"sv)); qaction.setShortcut(QKeySequence(Qt::CTRL | Qt::Key_0)); break; - case WebView::ActionID::DumpSessionHistoryTree: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/history.png"sv)); - break; - case WebView::ActionID::DumpDOMTree: - qaction.setIcon(load_icon_from_uri("resource://icons/browser/dom-tree.png"sv)); - break; - case WebView::ActionID::DumpLayoutTree: - case WebView::ActionID::DumpPaintTree: - case WebView::ActionID::DumpDisplayList: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/layout.png"sv)); - break; - case WebView::ActionID::DumpStackingContextTree: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/layers.png"sv)); - break; - case WebView::ActionID::DumpStyleSheets: - case WebView::ActionID::DumpStyles: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/filetype-css.png"sv)); - break; - case WebView::ActionID::DumpCSSErrors: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/error.png"sv)); - break; - case WebView::ActionID::DumpCookies: - qaction.setIcon(load_icon_from_uri("resource://icons/browser/cookie.png"sv)); - break; - case WebView::ActionID::DumpLocalStorage: - qaction.setIcon(load_icon_from_uri("resource://icons/browser/local-storage.png"sv)); - break; - case WebView::ActionID::ShowLineBoxBorders: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/box.png"sv)); - break; - case WebView::ActionID::CollectGarbage: - qaction.setIcon(load_icon_from_uri("resource://icons/16x16/trash-can.png"sv)); - break; - default: break; } @@ -300,7 +213,7 @@ static void initialize_native_control(WebView::Action& action, QAction& qaction, if (action.is_checkable()) qaction.setCheckable(true); - action.add_observer(ActionObserver::create(action, qaction)); + action.add_observer(ActionObserver::create(action, qaction, include_action_icon)); add_properties(qaction, action); } @@ -311,21 +224,13 @@ static void add_items_to_menu(QMenu& qmenu, QWidget& parent, WebView::Menu& menu for (auto& menu_item : menu.items()) { menu_item.visit( [&](NonnullRefPtr& action) { - auto* qaction = create_application_action(parent, action); + auto* qaction = create_application_action(parent, action, IncludeActionIcon::No); qmenu.addAction(qaction); - - if (action->id() == WebView::ActionID::SpoofUserAgent || action->id() == WebView::ActionID::NavigatorCompatibilityMode) { - if (qmenu.icon().isNull()) - qmenu.setIcon(load_icon_from_uri("resource://icons/16x16/spoof.png"sv)); - } }, [&](NonnullRefPtr const& submenu) { auto* qsubmenu = new QMenu(qstring_from_ak_string(submenu->title()), &qmenu); add_items_to_menu(*qsubmenu, parent, submenu); - if (submenu->render_group_icon()) - qsubmenu->setIcon(create_tvg_icon_with_theme_colors("folder", parent.palette())); - add_properties(*qsubmenu, *submenu); qmenu.addMenu(qsubmenu); }, @@ -360,10 +265,10 @@ QMenu* create_context_menu(QWidget& parent, WebContentView& view, WebView::Menu& return application_menu; } -QAction* create_application_action(QWidget& parent, WebView::Action& action) +QAction* create_application_action(QWidget& parent, WebView::Action& action, IncludeActionIcon include_action_icon) { auto* qaction = new QAction(&parent); - initialize_native_control(action, *qaction, parent.palette()); + initialize_native_control(action, *qaction, parent.palette(), include_action_icon); return qaction; } diff --git a/UI/Qt/Menu.h b/UI/Qt/Menu.h index ff759ff0fb..38a0fa0f64 100644 --- a/UI/Qt/Menu.h +++ b/UI/Qt/Menu.h @@ -16,10 +16,15 @@ namespace Ladybird { class WebContentView; +enum class IncludeActionIcon { + No, + Yes, +}; + QMenu* create_application_menu(QWidget& parent, WebView::Menu&); void repopulate_application_menu(QMenu& menu, QWidget& parent, WebView::Menu& source); QMenu* create_context_menu(QWidget& parent, WebContentView&, WebView::Menu&); -QAction* create_application_action(QWidget& parent, WebView::Action&); +QAction* create_application_action(QWidget& parent, WebView::Action&, IncludeActionIcon = IncludeActionIcon::Yes); } diff --git a/UI/Qt/Tab.cpp b/UI/Qt/Tab.cpp index e5b94f689b..98833505e6 100644 --- a/UI/Qt/Tab.cpp +++ b/UI/Qt/Tab.cpp @@ -495,7 +495,7 @@ Tab::Tab(BrowserWindow* window, RefPtr parent_client, }); m_context_menu = new QMenu("Context menu", this); - m_context_menu->addAction(create_application_action(*this, WebView::Application::the().reload_action())); + m_context_menu->addAction(create_application_action(*this, WebView::Application::the().reload_action(), IncludeActionIcon::No)); m_context_menu->addAction(duplicate_tab_action); m_context_menu->addSeparator(); auto* move_tab_menu = m_context_menu->addMenu("Mo&ve Tab"); @@ -684,7 +684,6 @@ void Tab::recreate_toolbar_icons() m_navigate_back_action->setIcon(create_chrome_icon(ChromeIcon::Back, palette())); m_navigate_forward_action->setIcon(create_chrome_icon(ChromeIcon::Forward, palette())); m_reload_action->setIcon(create_chrome_icon(ChromeIcon::Reload, palette())); - m_window->new_tab_action().setIcon(create_chrome_icon(ChromeIcon::NewTab, palette())); m_hamburger_button->setIcon(create_chrome_icon(ChromeIcon::Menu, palette())); if (auto* action = m_location_edit->trailing_action()) { auto icon = view().toggle_bookmark_action().engaged() ? ChromeIcon::StarFilled : ChromeIcon::Star;