fix(terminal): Ctrl+Click does not open URLs ending with a port number#552
Conversation
FullUrlRegExp's port group used a consuming boundary check [^0-9], which swallowed the character immediately after the port (newline, space, or '/') into the captured URL string. For URLs like 'http://localhost:8000' at end of a line, the capture became 'http://localhost:8000\n' and QUrl::StrictMode rejected it, so Ctrl+Click silently did nothing while the hover underline still showed. Replace [^0-9] with the non-consuming negative lookahead (?![0-9]). The boundary character is now asserted but not consumed, so the captured URL stays clean and QUrl accepts it. This also fixes a related issue where 'http://localhost:8000/path' was truncated to 'http://localhost:8000/' because the consumed '/' broke the optional path group. Verified against: http://localhost:8000 (was broken), http://localhost:8000/ (works), http://localhost:8000/path (was truncated), http://www.baidu.com:4443/, and invalid port boundary http://localhost:80000 (still rejected as expected).
|
CLA Assistant Lite bot: |
|
Hi @st0nie. Thanks for your PR. 😃 |
|
Hi @st0nie. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, st0nie The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Problem
In deepin-terminal, Ctrl + Left-click silently fails to open a URL when the URL ends with a port number and no trailing path/slash:
http://localhost:8000http://localhost:8000/http://www.baidu.com:4443/http://localhost:8000/pathhttp://localhost:8000/So the user sees a working underline on
http://localhost:8000, clicks it, and nothing happens — confusing and looks broken.Root Cause
UrlFilter::FullUrlRegExpin3rdparty/terminalwidget/lib/Filter.cppvalidates the port number with a consuming boundary check,[^0-9]:[^0-9]matches and consumes the first character after the port (the newline at end of line, a space, or a/). That character becomes part of the captured URL string. Forhttp://localhost:8000printed on its own line, the capture ishttp://localhost:8000\n.UrlFilter::HotSpot::activate()then hands the string toQUrl(url, QUrl::StrictMode), which rejects any URL containing a control character like\n. The open action is dropped silently — no error, no browser.The trailing-slash case
http://localhost:8000/happens to work because the consumed/still leaves a syntactically valid URL.The same consumed
/also explains the truncation in the last row: the optional path group([/][\w.@?^=%&/~+#-]+)?can no longer match because the leading/was already eaten by[^0-9].Fix
Replace the consuming character class
[^0-9]with a non-consuming negative lookahead(?![0-9]):The lookahead asserts the port is not followed by another digit (so
80000is still rejected as an out-of-range port) without swallowing the boundary character. The captured URL stays clean andQUrl::StrictModeaccepts it. As a side effect, the trailing-path case also stops being truncated because the/is left available for the path group.Verification
Tested with a Python equivalent of the regex (PCRE2-compatible, which is what
QRegularExpressionuses):echo http://localhost:8000\n(the bug)http://localhost:8000\n❌http://localhost:8000✅echo http://localhost:8000/ \nhttp://localhost:8000/http://localhost:8000✅echo http://localhost:8000/path\nhttp://localhost:8000/❌ truncatedhttp://localhost:8000/path✅echo http://www.baidu.com:4443/\nhttp://www.baidu.com:4443/http://www.baidu.com:4443✅echo http://localhost:80000\n(invalid port)http://localhost(boundary still rejects)http://localhost(boundary still rejects) ✅Invalid-port handling (
80000, out-of-range 0–65535) is preserved — the lookahead still fails to match and the port group is dropped, identical to the previous behavior.Scope of Change
A single one-character-class replacement in
3rdparty/terminalwidget/lib/Filter.cpp. No behavior change for valid URLs without ports, for email addresses, or for invalid port numbers.How to Reproduce
echo http://localhost:8000in deepin-terminalecho http://localhost:8000/— Ctrl+Click opens the browserAfter this patch, step 3 opens the browser as expected.