fix(term): provide linkHandler so OSC 8 hyperlinks open on Cmd/Ctrl-c…#3242
fix(term): provide linkHandler so OSC 8 hyperlinks open on Cmd/Ctrl-c…#3242s-zx wants to merge 1 commit intowavetermdev:mainfrom
Conversation
…lick xterm.js falls back to window.confirm() + window.open() for OSC 8 hyperlinks when no linkHandler is set. In Electron's renderer, window.open() navigation is intercepted by shNavHandler / shFrameNavHandler — so the dialog appears but the browser never opens. Route OSC 8 clicks through the same Cmd/Ctrl-click path that WebLinksAddon uses for regex-detected URLs.
|
s-zx seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
WalkthroughThis change adds a custom linkHandler override to the xterm terminal configuration within TermWrap construction. The handler intercepts OSC 8 hyperlink events, prevents default behavior, and implements custom link activation requiring platform-specific modifier keys (Meta on macOS, Ctrl elsewhere) before asynchronously opening links. It also introduces hover state tracking that updates Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/app/view/term/termwrap.ts (1)
146-211: Extract shared link activation/hover/leave helpers to remove duplication.The
linkHandlerblock (Lines 151-175) and theWebLinksAddoncallbacks (Lines 184-209) implement identical activate/hover/leave behavior. Extracting shared helpers keeps the two code paths in sync if the modifier-key policy or hover behavior changes later.♻️ Proposed refactor
+ const activateLink = (event: MouseEvent, uri: string) => { + event.preventDefault(); + const modPressed = PLATFORM === PlatformMacOS ? event.metaKey : event.ctrlKey; + if (modPressed) { + fireAndForget(() => openLink(uri)); + } + }; + const hoverLink = (event: MouseEvent, uri: string) => { + this.hoveredLinkUri = uri; + this.onLinkHover?.(uri, event.clientX, event.clientY); + }; + const leaveLink = () => { + this.hoveredLinkUri = null; + this.onLinkHover?.(null, 0, 0); + }; options.linkHandler = { - activate: (event: MouseEvent, uri: string) => { - event.preventDefault(); - switch (PLATFORM) { - case PlatformMacOS: - if (event.metaKey) { - fireAndForget(() => openLink(uri)); - } - break; - default: - if (event.ctrlKey) { - fireAndForget(() => openLink(uri)); - } - break; - } - }, - hover: (event: MouseEvent, uri: string) => { - this.hoveredLinkUri = uri; - this.onLinkHover?.(uri, event.clientX, event.clientY); - }, - leave: () => { - this.hoveredLinkUri = null; - this.onLinkHover?.(null, 0, 0); - }, + activate: activateLink, + hover: hoverLink, + leave: leaveLink, }; this.terminal = new Terminal(options); ... - this.terminal.loadAddon( - new WebLinksAddon( - (e, uri) => { ... }, - { - hover: (e, uri) => { ... }, - leave: () => { ... }, - } - ) - ); + this.terminal.loadAddon(new WebLinksAddon(activateLink, { hover: hoverLink, leave: leaveLink }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/view/term/termwrap.ts` around lines 146 - 211, The activate/hover/leave logic is duplicated between the options.linkHandler object and the WebLinksAddon callbacks; extract shared helpers (e.g., handleLinkActivate(event, uri), handleLinkHover(event, uri), handleLinkLeave()) and replace the inline implementations in both options.linkHandler and the WebLinksAddon initializer to call those helpers; ensure handleLinkActivate applies the PLATFORM-based modifier-key check (metaKey on PlatformMacOS, ctrlKey otherwise) and calls fireAndForget(() => openLink(uri)) when allowed, and ensure handleLinkHover/handleLinkLeave set this.hoveredLinkUri and call this.onLinkHover with the same arguments as the originals.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/view/term/termwrap.ts`:
- Around line 167-174: The hover/leave callbacks are duplicated in WebLinksAddon
options and terminal.options.linkHandler causing double calls; create a single
shared handler object (e.g., const linkHandler = { hover: (event, uri) => {
this.hoveredLinkUri = uri; this.onLinkHover?.(uri, event.clientX,
event.clientY); }, leave: () => { this.hoveredLinkUri = null;
this.onLinkHover?.(null, 0, 0); } }) then set terminal.options.linkHandler =
linkHandler and pass that same linkHandler to new
WebLinksAddon(activateCallback, linkHandler) instead of providing inline
hover/leave functions in two places (references: hoveredLinkUri, onLinkHover,
terminal.options.linkHandler, WebLinksAddon, hover, leave).
---
Nitpick comments:
In `@frontend/app/view/term/termwrap.ts`:
- Around line 146-211: The activate/hover/leave logic is duplicated between the
options.linkHandler object and the WebLinksAddon callbacks; extract shared
helpers (e.g., handleLinkActivate(event, uri), handleLinkHover(event, uri),
handleLinkLeave()) and replace the inline implementations in both
options.linkHandler and the WebLinksAddon initializer to call those helpers;
ensure handleLinkActivate applies the PLATFORM-based modifier-key check (metaKey
on PlatformMacOS, ctrlKey otherwise) and calls fireAndForget(() =>
openLink(uri)) when allowed, and ensure handleLinkHover/handleLinkLeave set
this.hoveredLinkUri and call this.onLinkHover with the same arguments as the
originals.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b4a158e4-ba9c-4bfd-8c39-298b1bf620c4
📒 Files selected for processing (1)
frontend/app/view/term/termwrap.ts
| hover: (event: MouseEvent, uri: string) => { | ||
| this.hoveredLinkUri = uri; | ||
| this.onLinkHover?.(uri, event.clientX, event.clientY); | ||
| }, | ||
| leave: () => { | ||
| this.hoveredLinkUri = null; | ||
| this.onLinkHover?.(null, 0, 0); | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
xterm.js linkHandler hover interaction with WebLinksAddon link provider ordering OSC 8
💡 Result:
In xterm.js, link detection and hover/click interactions for both WebLinksAddon (pattern-matched URLs) and OSC 8 hyperlinks are handled through the unified link provider system via terminal.registerLinkProvider. Providers are queried in registration order for links on a line; higher-priority (earlier registered) providers can suppress lower ones if ranges overlap. OSC 8 links are detected by the core OscLinkProvider (always active, detects via extended cell attributes set by OSC 8 escape sequences). WebLinksAddon registers a custom ILinkProvider for regex-matched web URLs. To control provider ordering: - Load WebLinksAddon first if you want pattern links to take priority over OSC 8. - Load custom providers or WebLinksAddon after core initialization to place them after OscLinkProvider. - Use terminal.options.linkHandler = {..., hover: (event, text, range) => {...}} for shared hover/activate callbacks across all providers (used by OscLinkProvider if set). Hover interaction: Implement linkHandler.hover(event, text, range) to show tooltips (e.g., full OSC 8 URI on hover, as display text may differ). Add 'xterm-hover' class to DOM elements to block event passthrough. Example from docs shows popup with URI and modifier hint. Setup example for both: const linkHandler = { hover: showLinkPopup, // Custom hover tooltip leave: removeLinkPopup, activate: (e, text) => { /* handle click */ }, allowNonHttpProtocols: true // For non-http OSC 8 }; terminal.options.linkHandler = linkHandler; terminal.loadAddon(new WebLinksAddon(undefined, { hover: linkHandler.hover, leave: linkHandler.leave })); This ensures consistent hover behavior. Registration order determines which provider "wins" overlapping ranges.
Citations:
- 1: http://xtermjs.org/docs/guides/link-handling/
- 2: http://xtermjs.org/docs/api/terminal/interfaces/ilinkhandler/
- 3: http://xtermjs.org/docs/api/terminal/classes/terminal/
- 4: Generic link provider xtermjs/xterm.js#2893
- 5: https://xtermjs.org/docs/guides/link-handling/
- 6: https://xtermjs.org/docs/api/terminal/interfaces/ilinkprovider
- 7: https://cdn.jsdelivr.net/npm/@xterm/xterm@6.0.0/src/browser/OscLinkProvider.ts
🏁 Script executed:
# Search for WebLinksAddon and linkHandler usage in the termwrap file and related files
rg -A 5 -B 5 "WebLinksAddon|linkHandler" frontend/app/view/term/termwrap.tsRepository: wavetermdev/waveterm
Length of output: 2054
🏁 Script executed:
# Look for the broader context around the hover/leave code
sed -n '150,190p' frontend/app/view/term/termwrap.tsRepository: wavetermdev/waveterm
Length of output: 1720
🏁 Script executed:
# Search for onLinkHover definition and usage
rg "onLinkHover" frontend/app/view/term/Repository: wavetermdev/waveterm
Length of output: 774
🏁 Script executed:
# Get more context around the WebLinksAddon initialization
sed -n '180,220p' frontend/app/view/term/termwrap.tsRepository: wavetermdev/waveterm
Length of output: 1714
🏁 Script executed:
# Search for the complete WebLinksAddon constructor call
rg -A 20 "new WebLinksAddon" frontend/app/view/term/termwrap.tsRepository: wavetermdev/waveterm
Length of output: 882
🌐 Web query:
xterm WebLinksAddon hover leave options vs terminal linkHandler both fire
💡 Result:
Yes, both the WebLinksAddon hover/leave options and the terminal.options.linkHandler hover/leave callbacks can fire for the same link interaction. This occurs when using both mechanisms together, as recommended in the official xterm.js documentation for handling both pattern-matched web links (via WebLinksAddon) and OSC 8 hyperlinks (via terminal.options.linkHandler). The documentation provides an example where the same linkHandler object (with shared hover and leave functions) is passed to WebLinksAddon(activateLink, linkHandler) and set as xterm.options.linkHandler = linkHandler. In this setup: - WebLinksAddon uses its ILinkProviderOptions.hover/leave (from the options parameter) for URL pattern links. - terminal.options.linkHandler (ILinkHandler.hover/leave) handles OSC 8 links. If separate handlers are implemented with their own hover/leave logic (e.g., both showing popups), the callbacks will execute independently for applicable links, causing both to fire on hover/leave events. This is by design for comprehensive link support across link types, but requires careful implementation to avoid duplicate effects like multiple popups. To prevent duplication: - Use a single shared handler object for both, as in the docs. - Implement idempotent hover/leave logic (e.g., remove previous popup before creating new). - The docs example demonstrates popup management in shared hover/leave functions. No bugs reported for duplicate firing; it's expected behavior when both are active.
Citations:
- 1: http://xtermjs.org/docs/guides/link-handling/
- 2: http://xtermjs.org/docs/api/terminal/interfaces/ilinkhandler/
- 3: https://github.com/xtermjs/xterm.js/blob/master/addons/addon-web-links/typings/addon-web-links.d.ts
- 4: https://xtermjs.org/docs/guides/link-handling/
🌐 Web query:
@xterm/addon-web-links constructor hover leave options override terminal linkHandler
💡 Result:
The @xterm/addon-web-links provides the WebLinksAddon class with a constructor that accepts an optional click handler and options for customizing hover, leave, and regex behaviors. There is no direct "linkHandler" parameter in the constructor; instead, customization is done via ILinkProviderOptions or by setting terminal.options.linkHandler separately for broader link handling (including OSC 8 links). ## Constructor typescript new WebLinksAddon(handler?: (event: MouseEvent, uri: string) => void, options?: ILinkProviderOptions) - handler: Optional callback fired on link activation (click). Defaults to opening the URI. - options: Optional ILinkProviderOptions object. ## ILinkProviderOptions Interface typescript interface ILinkProviderOptions { hover?: (event: MouseEvent, text: string, location: IViewportRange) => void; leave?: (event: MouseEvent, text: string) => void; urlRegex?: RegExp; } - hover: Callback when mouse hovers over a detected link. Use to show tooltips (create elements inside Terminal.element with class 'xterm-hover' to block events). - leave: Callback when mouse leaves a link (may fire before hover). - urlRegex: Custom RegExp to override default URL detection. ## Usage Example typescript import { Terminal } from '@xterm/xterm'; import { WebLinksAddon } from '@xterm/addon-web-links'; const term = new Terminal; const addon = new WebLinksAddon( (event, uri) => console.log('Clicked:', uri), // Custom click handler { hover: (event, text, location) => { // Show tooltip example from docs console.log('Hover:', text); }, leave: (event, text) => { console.log('Leave:', text); }, urlRegex: /custom-url-pattern/ // Override regex } ); term.loadAddon(addon); ## Overriding/Advanced linkHandler For comprehensive control (web-links + OSC 8), implement ILinkHandler and set term.options.linkHandler: typescript term.options.linkHandler = { activate: (event, text) => { /* handle click / }, hover: (event, text, range) => { / hover / }, leave: (event, text, range) => { / leave */ }, allowNonHttpProtocols: true // Optional, security risk if misused }; This works alongside the addon. Use modifier keys (Ctrl/Cmd+click) for safety. The addon focuses on regex-detected web links; terminal.options.linkHandler covers all links including explicit hyperlinks.
Citations:
- 1: https://github.com/xtermjs/xterm.js/blob/master/addons/addon-web-links/typings/addon-web-links.d.ts
- 2: https://xtermjs.org/docs/guides/link-handling/
- 3: https://xtermjs.org/docs/api/terminal/interfaces/ilinkhandler
Remove duplicate hover/leave callbacks from WebLinksAddon options.
The same hover and leave logic is implemented both in terminal.options.linkHandler and in the WebLinksAddon options. Per xterm.js documentation, for WebLinksAddon links, both will fire independently—causing the same state updates (hoveredLinkUri, onLinkHover) to be called twice. The recommended pattern is to pass a single shared handler object to both: set terminal.options.linkHandler = linkHandler, then pass that same handler to WebLinksAddon(activateCallback, linkHandler). This avoids duplication and ensures consistent behavior across link types.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/view/term/termwrap.ts` around lines 167 - 174, The hover/leave
callbacks are duplicated in WebLinksAddon options and
terminal.options.linkHandler causing double calls; create a single shared
handler object (e.g., const linkHandler = { hover: (event, uri) => {
this.hoveredLinkUri = uri; this.onLinkHover?.(uri, event.clientX,
event.clientY); }, leave: () => { this.hoveredLinkUri = null;
this.onLinkHover?.(null, 0, 0); } }) then set terminal.options.linkHandler =
linkHandler and pass that same linkHandler to new
WebLinksAddon(activateCallback, linkHandler) instead of providing inline
hover/leave functions in two places (references: hoveredLinkUri, onLinkHover,
terminal.options.linkHandler, WebLinksAddon, hover, leave).
2 participants
…lick
xterm.js falls back to window.confirm() + window.open() for OSC 8 hyperlinks when no linkHandler is set. In Electron's renderer, window.open() navigation is intercepted by shNavHandler / shFrameNavHandler — so the dialog appears but the browser never opens.
Route OSC 8 clicks through the same Cmd/Ctrl-click path that WebLinksAddon uses for regex-detected URLs.