Uri handling#542
Open
epicleafies wants to merge 30 commits into
Open
Conversation
Add an m_initialized flag so that initialize() is idempotent. main.qml now calls startNodeInitializionThread() before pushing the wallet creation wizard so the wallet loader is ready by the time the user reaches the password step. Without the guard a second call would enqueue a duplicate init task on the worker thread.
setSatoshi() only emitted amountChanged, so callers that read the formatted display string (e.g. the amount input field in Send.qml) did not update when a URI import set the amount directly. Emit displayChanged as well so those bindings re-evaluate.
Connect QClipboard::dataChanged to a new QML-accessible signal on the Clipboard singleton. The Send page uses this to detect when a bitcoin: URI is copied so it can show the clipboard import banner automatically.
Implement a standalone BIP21 bitcoin: URI parser in
qml/models/bitcoinuri.{h,cpp}. The parser validates the address
via DecodeDestination, handles the standard amount/label/message
query parameters with full percent-decoding (QUrl::FullyDecoded),
rejects bitcoin:// double-slash URIs, and honours the req- prefix
semantics by failing on unknown required parameters.
Add 17 unit tests covering valid URIs, all rejection cases, edge
cases (zero amount, empty amount param, duplicate amount, uppercase
scheme, legacy P2PKH address) and req- behaviour.
Add two Q_INVOKABLE methods: - parseBitcoinUri(uri_text): parse a bitcoin: URI from a string - parseBitcoinUriFromFile(source_path): read a local file and parse its contents as a URI; accepts both plain paths and file:// URLs (using QUrl::toLocalFile for cross-platform correctness) Both return a QVariantMap with success/error/address/amountSats/ hasAmount/label/hasLabel/uriMessage/hasMessage keys. The key is named "uriMessage" rather than "message" to avoid shadowing JavaScript's built-in Error.message property in QML error-handling code. File reads are capped at 1 MiB and performed synchronously on the GUI thread, which is acceptable for local storage.
Add a non-toggle companion to EllipsisMenuToggleItem for one-shot actions in ellipsis menus. The item matches the toggle's dimensions (280×44 px) and hover behaviour (orange text on hover) but does not carry a checkable state. Register it in bitcoin_qml.qrc.
Both controls hard-coded font.pixelSize: 18. Add a fontSize property (default 18, preserving existing behaviour) so callers can use compact button variants — e.g. the clipboard URI banner uses fontSize: 14 to fit Fill/Dismiss buttons in a tight row layout.
IconButton: - Replace MouseArea (enabled:false, hoverEnabled:true) with a HoverHandler; the old pattern leaked a transparent overlay that sometimes intercepted clicks in parent items. - Change default iconColor to neutral5 so idle icon buttons are visually subdued; they still animate to orange on hover/active. - Disable hoverEnabled on the inner Icon to avoid double state changes. - Add a PRESSED state so the background highlights on click the same way it does when the button is checked. EllipsisMenuToggleItem: - Add a fixed implicitHeight of 44 px to match EllipsisMenuButtonItem. - Remove the bgHoverColor/textColor/textHoverColor/textActiveColor properties that were only used internally; simplify the HOVER state to just set the text orange (consistent with EllipsisMenuButtonItem).
Add an EllipsisMenuButtonItem at the top of the Send options popup that emits an openPaymentRequest() signal. The Send page listens for this signal to open the manual URI entry popup. Switch the popup height to content-driven (columnLayout.implicitHeight + padding) so it grows automatically with new entries. Anchor the column to the popup edges rather than centering it.
DesktopWallets is pushed to the stack behind the CreateWalletWizard. Because StackView lazy-loads hidden items, its Component.onCompleted (which calls startNodeInitializionThread) does not fire until the wizard is dismissed — too late for wallet creation to succeed. Fix by calling nodeModel.startNodeInitializionThread() explicitly before pushing the wizard in the onboarding path in main.qml. The new QmlInitExecutor guard (see earlier commit) makes the call idempotent if DesktopWallets later fires its own Component.onCompleted. Also gate the Skip and Continue buttons in CreatePassword.qml on walletController.initialized so they cannot be clicked before the wallet loader is ready, preventing a crash from a premature createSingleSigWallet call.
Add objectName properties to the wallet creation wizard pages (CreateIntro, CreateName, CreatePassword, CreateConfirm, CreateBackup) and to their primary action buttons, plus the Activity and Send tabs in DesktopWallets. These names allow the functional test bridge to locate and interact with specific items by name without relying on fragile positional selectors.
Add four ways to import a BIP21 bitcoin: payment URI into the Send form: 1. Clipboard banner — shown automatically when a valid bitcoin: URI is detected in the clipboard (via the Clipboard.dataChanged signal). Fill/Dismiss buttons let the user apply or ignore it. 2. Manual popup — the "Open payment request" entry in the Send options menu opens a modal dialog where the user can paste or type a URI. 3. File import — via a DropArea that handles text/uri-list drops with file:// URLs; the C++ layer converts them to local paths using QUrl::toLocalFile(). 4. Drag-and-drop — the same DropArea also accepts text/plain drops (raw URIs dragged from another app) and non-file URLs. Successful import populates the address, amount, and note fields and shows a status row. Failed import shows an error in the same row. The "message=" field from the URI is displayed separately above the status row. Test automation hooks (hidden CoreTextFields + invisible Buttons) are conditionally loaded inside a Loader gated on the testAutomationEnabled context property (set in bitcoin.cpp at startup). In production builds the Loader is inactive and contributes nothing to the object tree.
TestBridge gains a set_clipboard_text command so functional tests can set the system clipboard contents without depending on platform-level input injection. QmlDriver.wait_for_property() is extended to support three matching modes beyond exact-value equality: contains= (substring), nonEmpty (truthy string), and value=None (property exists). This avoids brittle full-string matches for status messages that vary by import source. QmlDriver gains set_clipboard_text() and simulate_drop() helpers. QmlTestHarness gains pick_unused_port() to bind the test node's RPC port dynamically (avoiding conflicts when multiple test instances run in parallel) and process_output() to capture GUI stdout/stderr for failure diagnostics.
Add qml_test_uri_import.py covering seven URI import scenarios: 1. Clipboard banner: set clipboard to a valid URI, banner appears, Fill button populates address/amount/label/message fields. 2. Manual popup: open via the Send options menu, type a URI, apply. 3. Malformed URI error: bitcoin:// is rejected; error shown in status row. 4. File import: write a URI to a temp file, trigger via automation hook. 5. Drag-drop (text): simulate drop of a plain URI string. 6. Drag-drop (file://): simulate drop of a file:// URL via automation hook. 7. Drag-drop (non-file URL): exercises the non-file URL branch of DropArea. The test drives the full wallet creation wizard before running URI tests so it can obtain a real regtest address via RPC. The RPC port is assigned dynamically by QmlTestHarness to prevent port conflicts. Register the test in the CI workflow.
Capitalise "Bitcoin" when used as a proper noun in prose, drop the colon (which belongs to the URI scheme syntax, not sentences), and replace the redundant "URI cannot be parsed." prefix with concise standalone messages that read naturally on their own: "Enter a Bitcoin payment URI." "Not a valid Bitcoin payment URI." "Invalid Bitcoin amount." "Unsupported required parameter: <key>" Update unit test assertions to match.
Qt's default Overlay.modal color is #80000000 (50% black). In dark mode the app background is black, making the dim layer invisible. Fix by setting a custom Overlay.modal on the ApplicationWindow that uses neutral9 (white in dark mode, black in light mode) at 15% opacity — giving a visible veil in both themes. This affects all modal popups in the app (ban peer, external link, URI import). Also align the URI import popup style with the ban peer popup pattern: - background: Theme.color.background with neutral3 border, radius 8 - anchors.centerIn: parent (matches ban peer and ExternalPopup)
A window-level Overlay.modal on ApplicationWindow inherits to every popup in the hierarchy, including any third-party or future popups that may want a different (or no) dim. Attach the dark dim rectangle directly to each popup that needs it instead — ExternalPopup, PeerDetails' ban confirmation, the URI import popup, and SendResult. Also correct anchors.centerIn for the URI import popup: using Overlay.overlay (the actual overlay item) rather than parent ensures the popup is centred relative to the full window, matching the behaviour of the other modal popups.
OptionPopup inherited non-zero padding from its Popup base, which added unintended spacing around the content. Set padding: 0 on OptionPopup so subclasses control their own internal layout. In SendOptionsPopup, remove the Separator between the copy-address item and the coin control toggle — the visual break adds clutter without aiding scannability. Reduce the height buffer from 15 to 10 to account for the removed separator height.
Polish the text and visual treatment of the URI import popup: - Shorten the heading to "Payment request" (was "Open payment request", which is an action label, not a title) - Rewrite the clipboard banner text to "There's a payment request on your clipboard." — cleaner and avoids the ambiguous "invoice" term - Replace the "Dismiss" OutlineButton with a borderless cross IconButton to reduce visual weight in the banner - Wrap the payment-request message and status rows in a lightly shaded Rectangle card so they read as a distinct info block rather than floating text lines - Style the status clear button to match: icon-only, no background, consistent size with the dismiss button
parseBitcoinUri and parseBitcoinUriFromFile have no dependency on wallet
state — they only need chain params, which are globally available after
baseInitialize(). Placing them on WalletQmlModel forces callers to guard
against a null wallet for an operation that does not require one.
Move both methods and BuildBitcoinUriResultMap into a new BitcoinUriModel
class (qml/models/bitcoinurimodel.{h,cpp}) and register it as the "BitcoinUri"
singleton in org.bitcoincore.qt 1.0, alongside Clipboard. QML callers now
use BitcoinUri.parseBitcoinUri() / BitcoinUri.parseBitcoinUriFromFile()
instead of wallet.parseBitcoinUri().
Also replace the C-style (qlonglong) cast in BuildBitcoinUriResultMap with
static_cast<qlonglong> for consistency with the rest of the codebase.
The previous code had two problems: the decode_error string from
DecodeDestination is an untranslated C++ internal message that bypasses
the tr() system and appears raw in the UI, and the decode_error.empty()
fallback ("URI cannot be parsed. Use a valid bitcoin: payment URI.") was
inconsistent with the concise phrasing used for all other error messages
in the parser.
Replace both branches with a single translated message: "Not a valid
Bitcoin address."
Track clipboard URI state with three flags: m_pendingClipboardUri (cached at detection time to avoid TOCTOU races on Fill), m_filledUri (soft suppress: hides banner while form still matches URI fields, re-shows when any specified field diverges), and m_dismissedUri (hard suppress: hides banner until clipboard changes to a different URI). Add field-change Connections on address, amount and label guarded by m_applyingUri so programmatic fills do not re-trigger checkClipboard() before m_filledUri is set by the Fill handler. Move m_applyingUri = false to after all field writes in applyParsedPaymentRequest for the same reason. Guard checkClipboard() against !root.visible and null wallet, and run it from onVisibleChanged so the banner appears when navigating to Send with a valid URI already in the clipboard. Clear all URI import state on wallet switch. Update parse call sites to use the BitcoinUri singleton.
The clipboard banner dismiss button and the payment request status clear button have no visible label. Add Accessible.name and Accessible.role so screen readers can announce their purpose.
Replace the disabled MouseArea cursor pattern with HoverHandler, which is the idiomatic Qt Quick Controls 2 approach used elsewhere in the tree (e.g. IconButton).
The HOVER state only highlights the background; icon colour stays at iconColor (neutral5) on hover and only transitions to activeColor (orange) on CHECKED or PRESSED. Add a comment to make this intent explicit.
8a43d7f to
35326f9
Compare
When a URI is applied via the manual entry popup (or drag-and-drop), and that URI happens to match what is currently on the clipboard, the clipboard URI banner should disappear — the same way it does when the user clicks Fill. Previously the banner would stay visible because only the Fill button set m_filledUri. Apply the same soft-suppression logic in applyPaymentRequestFromText: if the applied URI equals the current clipboard text, set m_filledUri and hide the banner. The banner will re-appear automatically if the user later edits any field that the URI populated, preserving the existing fill-suppression semantics.
MarnixCroes
reviewed
Apr 2, 2026
Contributor
MarnixCroes
left a comment
There was a problem hiding this comment.
nice
Just did a quick test, will test/review more carefully later.
A couple first observations (some maybe for follow up):
- Payment request imported from clipboard message is still displayed when you add another recipient
- nit:
- when you already have entered a URI, then enter an empty one (click apply while empty) the UI is confusing
- when all fields are cleared, the message should also disappear
The paymentRequestStatus and paymentRequestMessage banners are page-level properties but describe an import that applies to a specific recipient. When the user navigates to a different recipient (next/prev/add/remove), the banners remain visible on the new, empty form — making it appear as though the new recipient already has an imported payment request. Clear the banners on currentRecipientChanged so they are only shown for the recipient they describe. Also clear them when the address field is emptied, since the import result is no longer relevant once the address is gone.
Clicking Apply with an empty input field called
applyPaymentRequestFromText("") which showed a red error banner
alongside any previously imported payment request, creating a
confusing mixed state with valid form fields and a new error.
Prevent this by disabling Apply when the input is empty.
A URI message= value with no whitespace (e.g. a long unbroken string) caused the banner text to overflow the container since wrapMode: WordWrap has no break points. Cap display at 3 lines and elide the remainder with an ellipsis.
The declarative text binding on the amount input breaks permanently once the user types (standard QML behavior). Add a Connections handler for onDisplayChanged so the text updates when flipUnit() is called.
johnny9
added a commit
to johnny9/BitcoinCoreAppDevelopment
that referenced
this pull request
Apr 27, 2026
# Conflicts: # .github/workflows/gui-functional-tests.yml # qml/pages/wallet/Send.qml # test/CMakeLists.txt # test/functional/qml_driver.py # test/functional/qml_test_harness.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added URI parsing logic using BIP 21 standard (#523).
Added clipboard banner that allows pasting URI into the send page.
Added manual URI option in menu on send page.
Added support for file and text drop into send page.
Added unit tests that cover all BIP 21 parsing paths as well as functional tests.