Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ make -C depends -j"$(( $(nproc) - 1 ))" | tail 5
--enable-werror

# Build with parallel jobs (leaving one core free)
# NOTE: Individual object files cannot be built separately; always do a full build
make -j"$(( $(nproc) - 1 ))"
```

Expand Down
4 changes: 2 additions & 2 deletions src/qt/bitcoin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ static void SetupUIArgs(ArgsManager& argsman)
argsman.AddArg("-choosedatadir", strprintf(QObject::tr("Choose data directory on startup (default: %u)").toStdString(), DEFAULT_CHOOSE_DATADIR), ArgsManager::ALLOW_ANY, OptionsCategory::GUI);
argsman.AddArg("-custom-css-dir", "Set a directory which contains custom css files. Those will be used as stylesheets for the UI.", ArgsManager::ALLOW_ANY, OptionsCategory::GUI);
argsman.AddArg("-font-family", QObject::tr("Set the font family. Possible values: %1. (default: %2)").arg(Join(GUIUtil::getFonts(/*selectable_only=*/true), ", ")).arg(GUIUtil::FontRegistry::DEFAULT_FONT).toStdString(), ArgsManager::ALLOW_ANY, OptionsCategory::GUI);
argsman.AddArg("-font-scale", QObject::tr("Set a scale factor which gets applied to the base font size. Possible range %1 (smallest fonts) to %2 (largest fonts). (default: %3)").arg(-100).arg(100).arg(GUIUtil::FontRegistry::DEFAULT_FONT_SCALE).toStdString(), ArgsManager::ALLOW_ANY, OptionsCategory::GUI);
argsman.AddArg("-font-scale", QObject::tr("Set a scale factor which gets applied to the base font size. Possible range %1 (smallest fonts) to %2 (largest fonts). (default: %3)").arg(-50).arg(100).arg(GUIUtil::FontRegistry::DEFAULT_FONT_SCALE).toStdString(), ArgsManager::ALLOW_ANY, OptionsCategory::GUI);
argsman.AddArg("-font-weight-bold", QObject::tr("Set the font weight for bold texts. Possible range %1 to %2 (default: %3)").arg(0).arg(8).arg(GUIUtil::weightToArg(GUIUtil::FontRegistry::TARGET_WEIGHT_BOLD)).toStdString(), ArgsManager::ALLOW_ANY, OptionsCategory::GUI);
argsman.AddArg("-font-weight-normal", QObject::tr("Set the font weight for normal texts. Possible range %1 to %2 (default: %3)").arg(0).arg(8).arg(GUIUtil::weightToArg(GUIUtil::FontRegistry::TARGET_WEIGHT_NORMAL)).toStdString(), ArgsManager::ALLOW_ANY, OptionsCategory::GUI);
argsman.AddArg("-lang=<lang>", QObject::tr("Set language, for example \"de_DE\" (default: system locale)").toStdString(), ArgsManager::ALLOW_ANY, OptionsCategory::GUI);
Expand Down Expand Up @@ -747,7 +747,7 @@ int GuiMain(int argc, char* argv[])
}
// Validate/set font scale
if (gArgs.IsArgSet("-font-scale")) {
const int nScaleMin = -100, nScaleMax = 100;
const int nScaleMin = -50, nScaleMax = 100;
int nScale = gArgs.GetIntArg("-font-scale", GUIUtil::g_font_registry.GetFontScale());
if (nScale < nScaleMin || nScale > nScaleMax) {
QMessageBox::critical(nullptr, PACKAGE_NAME,
Expand Down
50 changes: 50 additions & 0 deletions src/qt/bitcoingui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
#include <QVBoxLayout>
#include <QWindow>

#include <algorithm>
#include <functional>

namespace {
Expand All @@ -85,6 +86,10 @@ constexpr int GOV_CYCLE_FRAME_MS{STATUSBAR_ICON_CYCLE_MS / (GOV_CYCLE_FRAME_COUN

// Per-frame interval for the spinner animation
constexpr int SPINNER_FRAME_MS{STATUSBAR_ICON_CYCLE_MS / SPINNER_FRAMES};

constexpr int FONT_SCALE_MIN{-50};
constexpr int FONT_SCALE_MAX{100};
constexpr int FONT_SCALE_SHORTCUT_STEP{3};
} // anonymous namespace

const std::string BitcoinGUI::DEFAULT_UIPLATFORM =
Expand Down Expand Up @@ -517,12 +522,27 @@ void BitcoinGUI::createActions()
m_mask_values_action->setStatusTip(tr("Mask the values in the Overview tab"));
m_mask_values_action->setCheckable(true);

m_zoom_in_action = new QAction(tr("Zoom &In"), this);
m_zoom_in_action->setShortcuts({QKeySequence(QKeySequence::ZoomIn), QKeySequence(tr("Ctrl+="))});
m_zoom_in_action->setStatusTip(tr("Increase the font size"));

m_zoom_out_action = new QAction(tr("Zoom &Out"), this);
m_zoom_out_action->setShortcuts({QKeySequence(QKeySequence::ZoomOut), QKeySequence(tr("Ctrl+_"))});
Comment on lines +526 to +530
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Disambiguate global zoom shortcuts from console bindings

These new window-level shortcuts reuse Ctrl++/Ctrl+= and Ctrl+-/Ctrl+_, which are already bound in RPCConsole::setButtonIcons() for console font resizing. In -disablewallet mode (or no-wallet builds), BitcoinGUI embeds RPCConsole as the central widget, so both shortcut sets live in the same window context and become ambiguous; users can get inconsistent behavior (global UI scale changes instead of, or competing with, console-only font resizing). This regression is introduced by adding the duplicate key sequences here without scoping them away from the console context.

Useful? React with 👍 / 👎.

m_zoom_out_action->setStatusTip(tr("Decrease the font size"));
Comment thread
PastaPastaPasta marked this conversation as resolved.

m_zoom_reset_action = new QAction(tr("Reset &Zoom"), this);
m_zoom_reset_action->setShortcut(QKeySequence(tr("Ctrl+0")));
m_zoom_reset_action->setStatusTip(tr("Reset the font size to default"));
Comment on lines +525 to +535
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Zoom actions remain enabled when -font-scale is overridden.

When adjustFontScale() bails out because -font-scale is overridden, these actions still appear enabled in the View menu and their shortcuts remain advertised. This turns the menu items into silent no-ops for users with overridden configs.

Consider disabling the actions when the option is overridden:

💡 Proposed fix (disable actions when overridden)

In setClientModel() after getting the optionsModel, add:

const bool font_scale_locked{optionsModel->isOptionOverridden("-font-scale")};
m_zoom_in_action->setEnabled(!font_scale_locked);
m_zoom_out_action->setEnabled(!font_scale_locked);
m_zoom_reset_action->setEnabled(!font_scale_locked);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/qt/bitcoingui.cpp` around lines 525 - 535, When adjustFontScale() exits
early because the -font-scale option is overridden, disable the zoom actions so
they aren't active no-ops: in setClientModel() after retrieving optionsModel
call optionsModel->isOptionOverridden("-font-scale") (or similar) and use that
boolean to call setEnabled(false) on m_zoom_in_action, m_zoom_out_action and
m_zoom_reset_action when the option is locked; this keeps the View menu and
shortcut hints in sync with the actual behavior.

Comment on lines +525 to +535
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 Nitpick: Zoom actions remain enabled when -font-scale is overridden on the command line

When -font-scale is set via CLI, adjustFontScale() silently returns at line 766-767 without feedback. The zoom menu items and their shortcuts remain visually enabled, creating silent no-ops. The AppearanceWidget already follows the pattern of disabling its font-scale slider when the option is overridden (appearancewidget.cpp:188). Applying the same pattern to the zoom actions in setClientModel() would keep the UI consistent.

source: ['coderabbit']


connect(quitAction, &QAction::triggered, this, &BitcoinGUI::quitRequested);
connect(aboutAction, &QAction::triggered, this, &BitcoinGUI::aboutClicked);
connect(aboutQtAction, &QAction::triggered, qApp, QApplication::aboutQt);
connect(optionsAction, &QAction::triggered, this, &BitcoinGUI::optionsClicked);
connect(showHelpMessageAction, &QAction::triggered, this, &BitcoinGUI::showHelpMessageClicked);
connect(showCoinJoinHelpAction, &QAction::triggered, this, &BitcoinGUI::showCoinJoinHelpClicked);
connect(m_zoom_in_action, &QAction::triggered, this, [this] { adjustFontScale(FONT_SCALE_SHORTCUT_STEP); });
connect(m_zoom_out_action, &QAction::triggered, this, [this] { adjustFontScale(-FONT_SCALE_SHORTCUT_STEP); });
connect(m_zoom_reset_action, &QAction::triggered, this, [this] { adjustFontScale(0); });

// Jump directly to tabs in RPC-console
connect(openInfoAction, &QAction::triggered, this, &BitcoinGUI::showInfo);
Expand Down Expand Up @@ -677,6 +697,11 @@ void BitcoinGUI::createMenuBar()
}
settings->addAction(optionsAction);

QMenu* view = appMenuBar->addMenu(tr("&View"));
view->addAction(m_zoom_in_action);
view->addAction(m_zoom_out_action);
view->addAction(m_zoom_reset_action);

QMenu* window_menu = appMenuBar->addMenu(tr("&Window"));

QAction* minimize_action = window_menu->addAction(tr("&Minimize"));
Expand Down Expand Up @@ -735,6 +760,31 @@ void BitcoinGUI::createMenuBar()
help->addAction(aboutQtAction);
}

void BitcoinGUI::adjustFontScale(int delta)
{
if (clientModel && clientModel->getOptionsModel() &&
clientModel->getOptionsModel()->isOptionOverridden("-font-scale")) {
return;
}
Comment on lines +765 to +768
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Menu zoom items remain enabled when -font-scale CLI override is active

When -font-scale is overridden via CLI, adjustFontScale() silently returns at line 767 without any user feedback. The View > Zoom In / Zoom Out / Reset Zoom menu items remain visually enabled and clickable, but do nothing. Users have no way to know why the controls don't work.

source: ['codex', 'claude']

Comment on lines +763 to +768
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Zoom actions stay enabled when -font-scale disables the feature

adjustFontScale() returns immediately when OptionsModel::isOptionOverridden("-font-scale") is true, so the new View > Zoom In / Zoom Out / Reset Zoom actions become silent no-ops. The Appearance widget already handles the same override by disabling its slider in AppearanceWidget::setModel(), so leaving the menu actions enabled makes the UI inconsistent and advertises shortcuts that cannot work. Disable the three zoom actions when the option is overridden so the menu state matches the actual behavior.

source: ['claude']

🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `src/qt/bitcoingui.cpp`:
- [SUGGESTION] lines 763-768: Zoom actions stay enabled when `-font-scale` disables the feature
  `adjustFontScale()` returns immediately when `OptionsModel::isOptionOverridden("-font-scale")` is true, so the new View > Zoom In / Zoom Out / Reset Zoom actions become silent no-ops. The Appearance widget already handles the same override by disabling its slider in `AppearanceWidget::setModel()`, so leaving the menu actions enabled makes the UI inconsistent and advertises shortcuts that cannot work. Disable the three zoom actions when the option is overridden so the menu state matches the actual behavior.


const int current_scale{GUIUtil::g_font_registry.GetFontScale()};
const int new_scale{delta == 0
? GUIUtil::FontRegistry::DEFAULT_FONT_SCALE
: std::clamp(current_scale + delta, FONT_SCALE_MIN, FONT_SCALE_MAX)};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor full font-scale range when applying zoom step

adjustFontScale() clamps every non-reset zoom change to [-30, 30], but startup accepts -font-scale values up to [-100, 100] (src/qt/bitcoin.cpp), so users who run with a configured scale outside [-30, 30] will see Ctrl++ or Ctrl+- snap the scale directly to 30/-30 instead of adjusting from their current value. This is a regression introduced by the shortcut path and can unexpectedly shrink/enlarge UI text and persist the wrong value.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be resolved

Comment thread
PastaPastaPasta marked this conversation as resolved.

if (new_scale == current_scale) {
return;
}

GUIUtil::g_font_registry.SetFontScale(new_scale);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Respect overridden font-scale before mutating runtime fonts

The shortcut handler updates g_font_registry unconditionally, so Ctrl++/Ctrl+-/Ctrl+0 can still change live font scale even when -font-scale is CLI-overridden; this bypasses the existing override behavior where the Appearance slider is disabled (AppearanceWidget::setModel, isOptionOverridden("-font-scale")). In deployments that pin font scale via config/args, the new shortcuts now violate that expectation and can also write conflicting GUI settings.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be resolved

GUIUtil::updateFonts();
updateWidth();

if (clientModel && clientModel->getOptionsModel()) {
clientModel->getOptionsModel()->setOption(OptionsModel::FontScale, new_scale);
Comment on lines +779 to +784
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Shortcut-driven font-scale changes do not update the Appearance slider

The new shortcut path writes the persisted value with clientModel->getOptionsModel()->setOption(OptionsModel::FontScale, new_scale), but OptionsModel::setOption() does not emit dataChanged; only OptionsModel::setData() does. AppearanceWidget binds ui->fontScaleSlider through QDataWidgetMapper, so if the options dialog is open while the user presses Ctrl++ / Ctrl+- / Ctrl+0, the live font scale changes but the mapped slider can stay stale until the dialog is rebuilt. Updating the model through setData() (or emitting dataChanged for FontScale) would keep the dialog synchronized with the new shortcut path.

source: ['claude']

🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `src/qt/bitcoingui.cpp`:
- [SUGGESTION] lines 779-784: Shortcut-driven font-scale changes do not update the Appearance slider
  The new shortcut path writes the persisted value with `clientModel->getOptionsModel()->setOption(OptionsModel::FontScale, new_scale)`, but `OptionsModel::setOption()` does not emit `dataChanged`; only `OptionsModel::setData()` does. `AppearanceWidget` binds `ui->fontScaleSlider` through `QDataWidgetMapper`, so if the options dialog is open while the user presses Ctrl++ / Ctrl+- / Ctrl+0, the live font scale changes but the mapped slider can stay stale until the dialog is rebuilt. Updating the model through `setData()` (or emitting `dataChanged` for `FontScale`) would keep the dialog synchronized with the new shortcut path.

}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

void BitcoinGUI::createToolBars()
{
#ifdef ENABLE_WALLET
Expand Down
6 changes: 6 additions & 0 deletions src/qt/bitcoingui.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ class BitcoinGUI : public QMainWindow
QAction* m_close_all_wallets_action{nullptr};
QAction* m_wallet_selector_action = nullptr;
QAction* m_mask_values_action{nullptr};
QAction* m_zoom_in_action{nullptr};
QAction* m_zoom_out_action{nullptr};
QAction* m_zoom_reset_action{nullptr};

QComboBox* m_wallet_selector = nullptr;

Expand Down Expand Up @@ -262,6 +265,9 @@ class BitcoinGUI : public QMainWindow
/** Update UI with latest network info from model. */
void updateNetworkState();

/** Apply a global font scale delta or reset when delta is 0. */
void adjustFontScale(int delta);

/** Regenerate all pre-cached governance clock pixmaps (e.g. after a theme change). */
void refreshGovernanceCycleIcons();

Expand Down
4 changes: 2 additions & 2 deletions src/qt/forms/appearancewidget.ui
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,10 @@
</size>
</property>
<property name="minimum">
<number>-30</number>
<number>-50</number>
</property>
<property name="maximum">
<number>30</number>
<number>100</number>
Comment on lines 185 to +189
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect every place that defines, clamps, persists, or applies font scale.
rg -n -C3 'SetFontScale\(|GetFontScale\(|DEFAULT_FONT_SCALE|OptionID::FontScale|FontScale|fontScaleSlider|FONT_SCALE_MIN|FONT_SCALE_MAX|-30|-100' src

Repository: dashpay/dash

Length of output: 50369


🏁 Script executed:

# Search for minimum font size checks and scale_steps modifications
rg -n 'MIN_FONT_SIZE|min.*font.*size|m_scale_steps|SetFontScale|GetScaledFontSize' src/qt/guiutil_font.h src/qt/guiutil_font.cpp --max-count=50

Repository: dashpay/dash

Length of output: 928


🏁 Script executed:

# Check if there's any validation or clamping of font_scale when it's set
rg -n -A 5 'void SetFontScale|SetFontScale\(' src/qt/ --max-count=50

Repository: dashpay/dash

Length of output: 2458


🏁 Script executed:

# Check the UI file to see if the slider has any other constraints
rg -n -B 5 -A 10 'fontScaleSlider' src/qt/forms/appearancewidget.ui | head -80

Repository: dashpay/dash

Length of output: 650


🏁 Script executed:

# Check if there's any minimum font size validation or constraints
rg -n 'pointSizeF|setPointSize|MIN|MINIMUM|clamp.*size' src/qt/guiutil_font.cpp | head -50

Repository: dashpay/dash

Length of output: 628


🏁 Script executed:

# Check git history or find if there was a previous version with -30..30
rg -n '\-30|\-100' src/qt/forms/appearancewidget.ui src/qt/guiutil_font.h src/qt/bitcoingui.cpp

Repository: dashpay/dash

Length of output: 185


🏁 Script executed:

# Check the DEFAULT_FONT_SCALE and see if there's documentation
rg -n 'DEFAULT_FONT_SCALE' src/qt/ -A 2 -B 2

Repository: dashpay/dash

Length of output: 3384


🏁 Script executed:

# Check if there's any clamping in the font rendering or if Qt has built-in protection
rg -n 'setPointSizeF' src/qt/guiutil_font.cpp -B 3 -A 3 | head -60

Repository: dashpay/dash

Length of output: 1280


🏁 Script executed:

# Check if there's any minimum font size handling or if Qt's setPointSizeF has built-in protection
rg -n 'pointSize.*0|font.*size.*0|MIN.*POINT|pointSizeF.*<' src/qt/guiutil_font.cpp

Repository: dashpay/dash

Length of output: 97


🏁 Script executed:

# Check the context around line 582 in guiutil_font.cpp
sed -n '575,590p' src/qt/guiutil_font.cpp

Repository: dashpay/dash

Length of output: 610


🏁 Script executed:

# Let's find the full context of where GetScaledFontSize is used and verify the calculation
sed -n '620,635p' src/qt/guiutil_font.cpp

Repository: dashpay/dash

Length of output: 710


🏁 Script executed:

# Verify by calculating what font size would result at the extremes
# Let's check if there are any tests for font scaling
rg -n 'font.*scale|FontScale.*-100|-30' src/test/ --max-count=20

Repository: dashpay/dash

Length of output: 764


🏁 Script executed:

# Let's verify the exact calculation by checking if there's a minimum size applied anywhere
rg -n 'std::max\|clamp\|min.*size' src/qt/guiutil_font.h src/qt/guiutil_font.cpp | head -30

Repository: dashpay/dash

Length of output: 38


🏁 Script executed:

# Let's verify the complete picture - check all the places where SetFontScale is called to see if there's any clamping
rg -n -B 3 'SetFontScale\(nScale\)|SetFontScale\(new_scale\)' src/qt/

Repository: dashpay/dash

Length of output: 805


🏁 Script executed:

# Check if AppearanceWidget's updateFontScale clamps or validates the input
sed -n '239,243p' src/qt/appearancewidget.cpp

Repository: dashpay/dash

Length of output: 189


AppearanceWidget::updateFontScale() must validate the slider range before passing to SetFontScale().

The expanded slider range of -100..100 creates an unsafe code path. While command-line argument parsing (bitcoin.cpp:750–757) and keyboard shortcut handling (bitcoingui.cpp:773) both validate and clamp the font scale before applying it, AppearanceWidget::updateFontScale() at line 241 passes the slider value directly to SetFontScale(nScale) with no bounds checking.

With m_font_scale = -100 and m_scale_steps = 0.01, the scaling formula produces: GetScaledFontSize(size) = size * (1 + (-100 * 0.01)) = size * 0 = 0, resulting in zero font size.

Add a clamp in AppearanceWidget::updateFontScale() to ensure the value stays within the valid range, matching the validation already present in bitcoingui.cpp:773:

void AppearanceWidget::updateFontScale(int nScale)
{
    constexpr int FONT_SCALE_MIN{-100};
    constexpr int FONT_SCALE_MAX{100};
    nScale = std::clamp(nScale, FONT_SCALE_MIN, FONT_SCALE_MAX);
    GUIUtil::g_font_registry.SetFontScale(nScale);
    GUIUtil::updateFonts();
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/qt/forms/appearancewidget.ui` around lines 185 - 189,
AppearanceWidget::updateFontScale currently passes the raw slider value to
GUIUtil::g_font_registry.SetFontScale, which can produce an invalid zero font
size for extreme values; fix it by clamping the incoming nScale to the valid
range before calling SetFontScale (use the same bounds as elsewhere, e.g.
constexpr FONT_SCALE_MIN = -100 and FONT_SCALE_MAX = 100 and apply std::clamp),
then call GUIUtil::g_font_registry.SetFontScale(nScale) and
GUIUtil::updateFonts().

</property>
<property name="singleStep">
<number>10</number>
Expand Down
6 changes: 4 additions & 2 deletions src/qt/guiutil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,11 @@ void setupAppearance(QWidget* parent, OptionsModel* model)
}
}

void AddButtonShortcut(QAbstractButton* button, const QKeySequence& shortcut)
void AddButtonShortcut(QAbstractButton* button, const QKeySequence& shortcut, Qt::ShortcutContext context)
{
QObject::connect(new QShortcut(shortcut, button), &QShortcut::activated, [button]() { button->animateClick(); });
auto* sc = new QShortcut(shortcut, button);
sc->setContext(context);
QObject::connect(sc, &QShortcut::activated, [button]() { button->animateClick(); });
}

bool parseBitcoinURI(const QUrl &uri, SendCoinsRecipient *out)
Expand Down
2 changes: 1 addition & 1 deletion src/qt/guiutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ namespace GUIUtil
* @param[in] button QAbstractButton to assign shortcut to
* @param[in] shortcut QKeySequence to use as shortcut
*/
void AddButtonShortcut(QAbstractButton* button, const QKeySequence& shortcut);
void AddButtonShortcut(QAbstractButton* button, const QKeySequence& shortcut, Qt::ShortcutContext context = Qt::WindowShortcut);

// Parse "dash:" URI into recipient object, return true on successful parsing
bool parseBitcoinURI(const QUrl &uri, SendCoinsRecipient *out);
Expand Down
24 changes: 13 additions & 11 deletions src/qt/rpcconsole.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,17 @@ RPCConsole::RPCConsole(interfaces::Node& node, QWidget* parent, Qt::WindowFlags

setButtonIcons();

// Register console font-size shortcuts once (not in setButtonIcons which
// re-runs on theme changes and would create duplicate QShortcut objects).
//: Main shortcut to increase the RPC console font size.
GUIUtil::AddButtonShortcut(ui->fontBiggerButton, tr("Ctrl++"), Qt::WidgetWithChildrenShortcut);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bind RPC console zoom shortcuts with window context

Using Qt::WidgetWithChildrenShortcut here scopes the shortcut to ui->fontBiggerButton (and similarly ui->fontSmallerButton), which means Ctrl++/Ctrl+- only fire when those buttons have focus. In normal RPC console use, focus is in lineEdit or messagesWidget, so the console font-resize shortcuts stop working; in -disablewallet mode this also lets the new global zoom actions take over instead of console-local resizing. Restoring a window-level shortcut context avoids this regression.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore window-scoped context for console zoom shortcuts

Binding the RPC console zoom shortcuts with Qt::WidgetWithChildrenShortcut limits activation to fontBiggerButton/fontSmallerButton focus, so Ctrl++ and Ctrl+- no longer work during normal console usage when focus is in lineEdit or messagesWidget. This regresses previous keyboard behavior from setShortcut(...), and in -disablewallet mode it also allows the new global zoom actions to take over instead of console-local font resizing.

Useful? React with 👍 / 👎.

//: Secondary shortcut to increase the RPC console font size.
GUIUtil::AddButtonShortcut(ui->fontBiggerButton, tr("Ctrl+="), Qt::WidgetWithChildrenShortcut);
Comment on lines +564 to +566
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use window-scoped context for RPC console zoom shortcuts

Binding these QShortcuts with Qt::WidgetWithChildrenShortcut on ui->fontBiggerButton means they are only active while that button (or its children) has focus; in normal RPC console usage focus is on lineEdit/messagesWidget, so Ctrl++ and Ctrl+- no longer resize console fonts from the keyboard. This is a user-visible regression of existing console shortcut behavior and is especially noticeable when typing commands.

Useful? React with 👍 / 👎.

//: Main shortcut to decrease the RPC console font size.
GUIUtil::AddButtonShortcut(ui->fontSmallerButton, tr("Ctrl+-"), Qt::WidgetWithChildrenShortcut);
//: Secondary shortcut to decrease the RPC console font size.
GUIUtil::AddButtonShortcut(ui->fontSmallerButton, tr("Ctrl+_"), Qt::WidgetWithChildrenShortcut);
Comment on lines +564 to +570
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Overlapping shortcuts between global zoom and RPC console font-size buttons

The new window-level zoom actions use QKeySequence::ZoomIn (Ctrl++) and Ctrl+= (line 526 of bitcoingui.cpp). The RPC console registers widget-level shortcuts for the same keys (Ctrl++, Ctrl+=, Ctrl+-, Ctrl+_) at WidgetWithChildrenShortcut context. When the RPC console has focus, Qt may raise an ambiguous-shortcut conflict since both a WindowShortcut and WidgetWithChildrenShortcut match the same key. The two actions also do different things: one changes the global app font scale, the other changes only the console font size.

source: ['claude']

Comment on lines +564 to +570
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: RPC console font shortcuts are scoped to the buttons instead of the console

GUIUtil::AddButtonShortcut() creates QShortcut objects parented to the passed button (new QShortcut(shortcut, button) in guiutil.cpp). With Qt::WidgetWithChildrenShortcut, a shortcut is only active while that parent widget or one of its children has focus. Because the new console bindings are attached to ui->fontBiggerButton / ui->fontSmallerButton, Ctrl++ and Ctrl+- will not fire while focus is in the line edit, message view, or the rest of the RPC console — exactly the places users normally are when they want these shortcuts. Parent the shortcuts to the console/dialog instead of the individual buttons so the advertised bindings are active across the RPC console window.

source: ['codex']

🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `src/qt/rpcconsole.cpp`:
- [SUGGESTION] lines 564-570: RPC console font shortcuts are scoped to the buttons instead of the console
  `GUIUtil::AddButtonShortcut()` creates `QShortcut` objects parented to the passed button (`new QShortcut(shortcut, button)` in `guiutil.cpp`). With `Qt::WidgetWithChildrenShortcut`, a shortcut is only active while that parent widget or one of its children has focus. Because the new console bindings are attached to `ui->fontBiggerButton` / `ui->fontSmallerButton`, Ctrl++ and Ctrl+- will not fire while focus is in the line edit, message view, or the rest of the RPC console — exactly the places users normally are when they want these shortcuts. Parent the shortcuts to the console/dialog instead of the individual buttons so the advertised bindings are active across the RPC console window.


// Install event filter for up and down arrow
ui->lineEdit->installEventFilter(this);
ui->lineEdit->setMaxLength(16 * 1024 * 1024);
Expand Down Expand Up @@ -1031,8 +1042,8 @@ void RPCConsole::clear(bool keep_prompt)
" without fully understanding the ramifications of a command.%8")
.arg(PACKAGE_NAME,
"<b>" + ui->clearButton->shortcut().toString(QKeySequence::NativeText) + "</b>",
"<b>" + ui->fontBiggerButton->shortcut().toString(QKeySequence::NativeText) + "</b>",
"<b>" + ui->fontSmallerButton->shortcut().toString(QKeySequence::NativeText) + "</b>",
"<b>" + QKeySequence(tr("Ctrl++")).toString(QKeySequence::NativeText) + "</b>",
"<b>" + QKeySequence(tr("Ctrl+-")).toString(QKeySequence::NativeText) + "</b>",
Comment on lines 1044 to +1046
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 Nitpick: Shortcut display text in clear() help message is hardcoded separately

The clear() method at line 1045-1046 displays Ctrl++ and Ctrl+- as help text using hardcoded QKeySequence(tr("Ctrl++")). The actual shortcuts are registered separately at lines 564-570. If either location is changed independently, the displayed shortcuts in the help text will be wrong.

source: ['claude']

"<b>help</b>",
"<b>help-console</b>",
"<span class=\"secwarning\">",
Expand Down Expand Up @@ -1347,16 +1358,7 @@ void RPCConsole::setButtonIcons()
GUIUtil::setIcon(ui->clearButton, "remove", GUIUtil::ThemedColor::RED, consoleButtonsSize);

GUIUtil::setIcon(ui->fontBiggerButton, "fontbigger", GUIUtil::ThemedColor::BLUE, consoleButtonsSize);
//: Main shortcut to increase the RPC console font size.
ui->fontBiggerButton->setShortcut(tr("Ctrl++"));
//: Secondary shortcut to increase the RPC console font size.
GUIUtil::AddButtonShortcut(ui->fontBiggerButton, tr("Ctrl+="));

GUIUtil::setIcon(ui->fontSmallerButton, "fontsmaller", GUIUtil::ThemedColor::BLUE, consoleButtonsSize);
//: Main shortcut to decrease the RPC console font size.
ui->fontSmallerButton->setShortcut(tr("Ctrl+-"));
//: Secondary shortcut to decrease the RPC console font size.
GUIUtil::AddButtonShortcut(ui->fontSmallerButton, tr("Ctrl+_"));
}

void RPCConsole::reloadThemedWidgets()
Expand Down
Loading