Handle external link and open file failures - #767
Open
epicleafies wants to merge 8 commits into
Open
Conversation
epicleafies
force-pushed
the
handle-url-failures
branch
2 times, most recently
from
June 30, 2026 16:20
5c63277 to
e4f8bec
Compare
epicleafies
force-pushed
the
handle-url-failures
branch
from
July 9, 2026 23:50
72abf63 to
2d238fc
Compare
epicleafies
force-pushed
the
handle-url-failures
branch
from
July 30, 2026 17:23
2d238fc to
dd4012e
Compare
The Version row on the About page opens an external download URL, like the Website, Source code, and License rows, but it overrides the link icon with a caret-right. That caret is the in-app navigation symbol used by rows that push another page (for example Developer options), so it wrongly signals an internal navigation for what is actually an external link, and it is inconsistent with the other three external links. Drop the caret-right override so the Version link uses the shared external-link (export) icon, making all four About external links consistent.
epicleafies
force-pushed
the
handle-url-failures
branch
2 times, most recently
from
July 31, 2026 20:23
f840195 to
5570294
Compare
Opening an external link ignored whether it actually succeeded. ExternalPopup called Qt.openUrlExternally() and closed immediately, and ExternalLink opened links directly, so when the OS or browser could not open the URL the user was left with no feedback and no way to recover. Give ExternalLink ownership of the confirmation dialog. A page can no longer wire a link up wrong, because there is no way to build a link that opens nothing: the control creates its own ExternalPopup on first click, names it after itself so several links on one page stay individually addressable from tests, and routes every open through it. ExternalPopup and Separator move into qml/controls alongside it, since a control cannot depend on qml/components. Route the open through a new UrlOpener singleton rather than calling Qt.openUrlExternally() from QML. That puts the http/https allowlist in C++ at the single point where the app hands a URL to the operating system, so a later caller cannot inherit an unchecked handoff to the handler registry, and it makes the outcome injectable, so both paths are testable without launching a browser while the shipped QML keeps one unconditional call path. On failure the dialog stays open, explains what happened, and offers to copy the URL. It also takes focus while open, without which Tab kept cycling the page behind the modal dialog instead of its own buttons, and entering the error state moves focus onto the copy button so the swap is not a visual-only change. Copying disables that button while it confirms, so the label always describes what the button will do and a repeat copy is not silently a no-op. The dialog now bounds its own width and centres itself rather than leaving that to each caller, which is what made it span the whole window on the activity details page, and it wraps the URL mid-token so a txid cannot spill past the dialog edge.
Add a bridge-driven functional test that reaches the About page from the onboarding cover, opens the external-link confirmation popup from a link row, and verifies that cancelling closes it without leaving the page. It deliberately never confirms the open, since that would hand the URL to a real handler and try to launch a browser; the success, failure, scheme rejection and copy-URL paths are covered by the QML unit test, which drives a stubbed UrlOpener. Register the test in the GUI functional-test workflow so it runs in CI.
ToastBanner starts fully transparent and only becomes opaque from its onVisibleChanged fade-in. That handler never runs for a banner whose condition already holds when it is built inside an already visible page: visible never changes, so the banner keeps taking up its space in the layout while drawing nothing. Re-entering the debug log page with the log still missing showed exactly that, a blank strip where the persistent error banner should be. Whether the fade-in runs depends on creation order. A page whose tree becomes visible only after construction still gets the visibility change and fades in, which is why every existing call site, which binds to state that starts empty, never hit this. Bind the initial opacity to the initial visibility so both orders draw the banner, and start the auto-dismiss countdown at creation for a banner shown this way, since the fade-in that normally arms it does not run.
DebugLogModel has always detected both ways the debug log can be unavailable: the background reader records when the file cannot be read, and openLogFile() records when it cannot be handed to another application. Neither was ever shown. SettingsDebugLog discarded openLogFile()'s return value and nothing read openError at all, so both failures were silent. Show them in a ToastBanner on the debug log page, matching the Sign/Verify message result banner styling. The two failures need separate fields because they have different lifetimes. The reader's error describes the state of the log and has to survive navigation, since that condition outlives any one visit. The open error is the outcome of a single click and must not, or a stale failure follows the user around; the page clears it on creation through a new clearOpenError(). Sharing one field also let the reader wipe the click failure on the next auto-refresh tick, leaving only a brief flash of the message. For the same reason openLogFile() no longer reports a missing file itself. A missing log is a state of the log rather than the outcome of a click, so reporting it as one let the page clear it on re-entry while the file was still missing. The reader owns that message and keeps it for as long as the condition holds. Add logAvailable so the page can disable what a missing log makes meaningless: searching its contents, and opening it in another application. Refresh stays enabled, since it is how the user recovers once the file exists again. Cover it both ways: QML tests check that the error shows on a failed open, clears on a later success, does not survive page re-creation, and that an unreadable log disables search and export; C++ unit tests check that the missing-file path is left to the reader, that the open failure survives a successful background read, that clearOpenError() drops it, and that logAvailable follows the reader in both directions.
ExternalLink only changed color when a surrounding row drove its parentState, so standalone instances such as the third-party transaction links on the activity details page gave no visual cue that they are clickable, and no instance showed a pointing hand cursor. Handle hover in the control itself so every ExternalLink highlights on hover and shows a pointing hand cursor, while a DISABLED parentState still suppresses both.
ToastBanner's text item had no textFormat set, so it defaulted to AutoText, which silently upgrades markup-looking strings to rich text. Banner text is always a message, never markup, and error messages can interpolate externally influenced strings such as filesystem paths, so render it as plain text.
The third-party transaction links were stretched to the full width of the column. ExternalLink lets its label take the free space and pins the icon to the trailing edge, which is right for the About rows, where the label belongs on the left of a full-width settings row. Here it left the label centred but the icon stranded at the far edge, and made the whole column width clickable rather than the link. Let each link size to its own content and centre it, so the icon sits next to the label it belongs to and the clickable area is the link.
epicleafies
force-pushed
the
handle-url-failures
branch
from
August 1, 2026 20:53
5570294 to
76715d1
Compare
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
1 participant
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.
Updated external-link opening so a failed open gives the user feedback instead of silently doing nothing. Fixes #597
Known limitation: the platform only reports whether a handler process was launched. A browser that starts and then fails still reads as success, so the error handling covers the no-handler-available case.
About page open externally fail pop up
Debug log page open externally fail banner
Activity details page link open pop-up