Open doc links in a new tab instead of navigating the sandboxed frame - #19
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Highlighted links still navigate frame
- Changed the document click link interceptor to capture phase so external links inside highlight segments are prevented before segment click handlers stop propagation.
Or push these changes by commenting:
@cursor push 042165efd6
Preview (042165efd6)
diff --git a/lib/docs/overlay.ts b/lib/docs/overlay.ts
--- a/lib/docs/overlay.ts
+++ b/lib/docs/overlay.ts
@@ -737,7 +737,7 @@
ev.preventDefault();
window.open(href, "_blank", "noopener");
}
- document.addEventListener("click", openLink);
+ document.addEventListener("click", openLink, true);
document.addEventListener("auxclick", openLink);
// click-elsewhere (on non-highlight) clears focus + selection popoversYou can send follow-ups to the cloud agent here.
|
Confirmed and fixed in 44cca03 — good catch. A link inside a painted highlight segment did still navigate the frame: the segment's own click handler Re-verified in the local harness with a link wrapped in a
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Chip clicks open enclosing links
- The capture-phase link interceptor now ignores overlay chrome targets, so chip clicks only toggle reactions and no longer open enclosing anchors.
Or push these changes by commenting:
@cursor push 467b38f9d2
Preview (467b38f9d2)
diff --git a/lib/docs/overlay.ts b/lib/docs/overlay.ts
--- a/lib/docs/overlay.ts
+++ b/lib/docs/overlay.ts
@@ -731,6 +731,7 @@
// by the segment's own click handler (it stopPropagation()s) and navigate.
function openLink(ev){
if (editing) return;
+ if (fromOverlayChrome(ev)) return;
if (ev.type === "auxclick" && ev.button !== 1) return; // middle-click only
var t = ev.target;
var a = t && t.closest && t.closest("a[href]");You can send follow-ups to the cloud agent here.
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Link clicks still focus highlights
- Added stopPropagation() in openLink so external-link clicks no longer bubble to highlight segment click handlers.
Or push these changes by commenting:
@cursor push fe0cbd0437
Preview (fe0cbd0437)
diff --git a/lib/docs/overlay.ts b/lib/docs/overlay.ts
--- a/lib/docs/overlay.ts
+++ b/lib/docs/overlay.ts
@@ -737,6 +737,7 @@
var href = a && externalHref(a);
if (!href) return;
ev.preventDefault();
+ ev.stopPropagation();
window.open(href, "_blank", "noopener");
}
document.addEventListener("click", openLink, true);You can send follow-ups to the cloud agent here.
|
Both fixed in Chip clicks open enclosing links (medium) — real, and worse than described: the interceptor ran first in capture, so a chip that lands inside an anchor lost its reaction toggle and opened a tab. Link clicks still focus highlights (low) — Re-verified with a real chip: the harness shell posts
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Selection click opens external tab
- Added a non-collapsed selection guard in
openLinkthat suppresses link navigation without opening a new tab when drag-selection ends on linked text.
- Added a non-collapsed selection guard in
Or push these changes by commenting:
@cursor push d2aad954ab
Preview (d2aad954ab)
diff --git a/lib/docs/overlay.ts b/lib/docs/overlay.ts
--- a/lib/docs/overlay.ts
+++ b/lib/docs/overlay.ts
@@ -727,6 +727,14 @@
if (u.href.split("#")[0] === location.href.split("#")[0]) return null;
return u.href;
}
+ function hasActiveSelection(){
+ try {
+ var sel = window.getSelection();
+ return !!(sel && sel.rangeCount && !sel.isCollapsed && sel.toString().trim());
+ } catch (e) {
+ return false;
+ }
+ }
// Capture phase: a link inside a highlighted span would otherwise be swallowed
// by the segment's own click handler (it stopPropagation()s) and navigate.
function openLink(ev){
@@ -746,6 +754,13 @@
var a = t.closest("a[href]");
var href = a && externalHref(a);
if (!href) return;
+ if (hasActiveSelection()){
+ // A drag-selection ending on linked text still emits click; keep the frame
+ // from navigating, but do not treat selection as link-follow.
+ ev.preventDefault();
+ ev.stopPropagation();
+ return;
+ }
// Following a link is navigation, not an annotation click: keep the segment
// handler from also focusing the rail / opening the anchor picker.
ev.preventDefault();You can send follow-ups to the cloud agent here.
|
Fixed in Selection click opens external tab (medium) — Also swapped the chip/popover check for the existing Verified in the harness with a live selection inside the anchor at click time:
(Chrome starts a link drag rather than a selection when the drag begins inside linked text, so that path never fires a click at all — the guard covers the shift-click / drag-into-link cases where a selection does survive.) |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Edit mode middle-click opens links
- When edit mode is active, the shared link handler now prevents default navigation for anchor targets before returning, which blocks auxclick middle-click tab opens as intended.
Or push these changes by commenting:
@cursor push 28093e92bd
Preview (28093e92bd)
diff --git a/lib/docs/overlay.ts b/lib/docs/overlay.ts
--- a/lib/docs/overlay.ts
+++ b/lib/docs/overlay.ts
@@ -730,7 +730,11 @@
// Capture phase: a link inside a highlighted span would otherwise be swallowed
// by the segment's own click handler (it stopPropagation()s) and navigate.
function openLink(ev){
- if (editing) return;
+ if (editing){
+ var et = ev.target;
+ if (et && et.closest && et.closest("a[href]")) ev.preventDefault();
+ return;
+ }
if (ev.type === "auxclick" && ev.button !== 1) return; // middle-click only
var t = ev.target;
if (!t || !t.closest) return;You can send follow-ups to the cloud agent here.
|
Fixed in Edit mode middle-click opens links (medium) — reproduced: with edit mode on and no block yet focused, a middle-click on a link opened a tab (once a block is contenteditable the browser stops following links, which is why it only shows up on the first gesture).
The reordering also dropped the duplicated |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Selection click still focuses highlights
- The live-selection guard in
openLinknow also stops propagation so drag-end clicks on linked highlights no longer bubble into segment focus handlers.
- The live-selection guard in
Or push these changes by commenting:
@cursor push 8a85ae5c60
Preview (8a85ae5c60)
diff --git a/lib/docs/overlay.ts b/lib/docs/overlay.ts
--- a/lib/docs/overlay.ts
+++ b/lib/docs/overlay.ts
@@ -752,6 +752,7 @@ export const OVERLAY_SCRIPT = String.raw`
var sel = window.getSelection();
if (sel && sel.rangeCount && !sel.isCollapsed && sel.toString().trim()){
ev.preventDefault();
+ ev.stopPropagation();
return;
}
// Following a link is navigation, not an annotation click: keep the segment
@@ -752,6 +752,7 @@ export const OVERLAY_SCRIPT = String.raw`
var sel = window.getSelection();
if (sel && sel.rangeCount && !sel.isCollapsed && sel.toString().trim()){
ev.preventDefault();
+ ev.stopPropagation();
return;
}
// Following a link is navigation, not an annotation click: keep the segmentYou can send follow-ups to the cloud agent here.
|
Fixed in Selection click still focuses highlights (medium) — right, the guard returned before Verified on a reacted (highlighted) span inside an anchor, with a live selection at click time:
So the comment/react toolbar still comes up and the rail no longer jumps. |
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Stale focus after link click
- External link handling now clears existing annotation focus immediately after suppressing frame navigation, so dimming and rail pin state do not persist.
- ✅ Fixed: Hash links ignore base URL
- Fragment hrefs are now resolved against document.baseURI before same-document checks, so base-retargeted hashes are treated as external navigation and opened out of frame.
Or push these changes by commenting:
@cursor push 64d4bc121e
Preview (64d4bc121e)
diff --git a/lib/docs/overlay.ts b/lib/docs/overlay.ts
--- a/lib/docs/overlay.ts
+++ b/lib/docs/overlay.ts
@@ -720,9 +720,9 @@
// Same-document fragments (#section, the gutter section links) stay in-frame.
function externalHref(a){
var raw = a.getAttribute("href");
- if (!raw || raw.charAt(0) === "#") return null;
+ if (!raw) return null;
var u;
- try { u = new URL(a.href, document.baseURI); } catch(e){ return null; }
+ try { u = new URL(raw, document.baseURI); } catch(e){ return null; }
if (u.protocol !== "http:" && u.protocol !== "https:") return null;
if (u.href.split("#")[0] === location.href.split("#")[0]) return null;
return u.href;
@@ -751,6 +751,7 @@
ev.preventDefault();
ev.stopPropagation();
hidePop();
+ if (focusKey) clearFocus();
// …but a selection that survives the click is a comment/react gesture (a drag
// that ended in linked text), so keep the selection and open nothing. Same
// emptiness test as reportSelection.You can send follow-ups to the cloud agent here.
|
Both fixed in Stale focus after link click (medium) — Hash links ignore base URL (low) — dropped the That change made the overlay's own gutter section links resolve externally too, so they're now treated as our chrome alongside chips and popovers — verified that a gutter click still sends Full sweep on the final code (no-base doc): plain link → tab; link inside a highlight → tab, no rail focus; chip inside a link → reaction toggles, no tab; live selection → nothing opens, selection kept; edit mode left/middle click → nothing opens; |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Interceptor blocks author link handlers
- I removed capture-phase propagation blocking for intercepted links and made segment click handlers skip default-prevented events so author link handlers still execute.
- ✅ Fixed: SVG links resolve to garbage URLs
- I now read link destinations from the element’s href attribute/baseVal and resolve them against baseURI before URL parsing, which correctly handles SVG anchors.
Or push these changes by commenting:
@cursor push bc5cf8fcd0
Preview (bc5cf8fcd0)
diff --git a/lib/docs/overlay.ts b/lib/docs/overlay.ts
--- a/lib/docs/overlay.ts
+++ b/lib/docs/overlay.ts
@@ -629,6 +629,7 @@
span.addEventListener("mouseleave", function(){ setHover(null); send({type:"jh:hlHoverOut"}); });
span.addEventListener("click", function(ev){
ev.stopPropagation();
+ if (ev.defaultPrevented) return;
onSegClick(seg, ev);
});
}
@@ -718,9 +719,22 @@
// ourselves from inside the user gesture (sandbox carries allow-popups +
// allow-popups-to-escape-sandbox so the target loads as a normal page).
// Same-document fragments (#section, the gutter section links) stay in-frame.
+ function linkHrefValue(a){
+ if (!a) return null;
+ var href = null;
+ try { href = a.getAttribute ? a.getAttribute("href") : null; } catch(e){}
+ if (!href){
+ var ah = a.href;
+ if (ah && typeof ah.baseVal === "string") href = ah.baseVal;
+ else if (typeof ah === "string") href = ah;
+ }
+ return href || null;
+ }
function externalHref(a){
+ var href = linkHrefValue(a);
+ if (!href) return null;
var u;
- try { u = new URL(a.href, document.baseURI); } catch(e){ return null; }
+ try { u = new URL(href, a.baseURI || document.baseURI); } catch(e){ return null; }
if (u.protocol !== "http:" && u.protocol !== "https:") return null;
if (u.href.split("#")[0] === location.href.split("#")[0]) return null;
return u.href;
@@ -747,9 +761,9 @@
// A click on a link is navigation, not an annotation click: the frame must not
// move, and the segment handler must not focus the rail / open the anchor picker.
ev.preventDefault();
- ev.stopPropagation();
- // Click-elsewhere semantics, which stopPropagation keeps the handler below from
- // applying: a click off the annotation UI drops the popover and pinned focus.
+ // Keep propagation alive so author click handlers on the link still run.
+ // Segment handlers ignore defaultPrevented events, so they won't treat this as
+ // an annotation click.
hidePop();
if (focusKey && !t.closest("[data-jh-seg]")) clearFocus();
// …but a selection that survives the click is a comment/react gesture (a dragYou can send follow-ups to the cloud agent here.
|
Both fixed in Interceptor blocks author link handlers (medium) — confirmed: an
SVG links resolve to garbage URLs (medium) —
Re-ran the whole sweep on the final code: plain link, link inside a highlight, chip inside a link, live selection, edit-mode left/middle click, gutter section link, |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Stale focus after in-highlight link
- I moved the segment click
stopPropagation()to run only for non-link-claimed clicks so external-link clicks inside highlights now bubble to the document click-elsewhere handler and clear stale focus/popovers.
- I moved the segment click
Or push these changes by commenting:
@cursor push 53907ec9be
Preview (53907ec9be)
diff --git a/lib/docs/overlay.ts b/lib/docs/overlay.ts
--- a/lib/docs/overlay.ts
+++ b/lib/docs/overlay.ts
@@ -628,9 +628,9 @@
});
span.addEventListener("mouseleave", function(){ setHover(null); send({type:"jh:hlHoverOut"}); });
span.addEventListener("click", function(ev){
- ev.stopPropagation();
// A click the link interceptor claimed is navigation, not an annotation click.
if (ev === linkClick) return;
+ ev.stopPropagation();
onSegClick(seg, ev);
});
}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 99c12f4. Configure here.
| var sel = window.getSelection(); | ||
| if (sel && sel.rangeCount && !sel.isCollapsed && sel.toString().trim()) return; | ||
| window.open(href, "_blank", "noopener"); | ||
| } |
There was a problem hiding this comment.
Stale focus after in-highlight link
Medium Severity
When an external link sits inside a highlight segment, openLink opens the tab and sets linkClick, but the segment handler still stopPropagation()s before bailing. That blocks the document click-elsewhere listener, so pinned focus/dimming and any open pick popover remain after the navigation gesture—neither onSegClick nor clearFocus/hidePop runs.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 99c12f4. Configure here.
1 participant
Problem
Clicking any external link inside a published document replaces the document with the browser's "refused to connect" error page.
The viewer serves user HTML from
/d/:slug/rawinto a sandboxed iframe (sandbox="allow-scripts allow-downloads", plus the matching CSPsandboxheader). A plain<a href="https://github.com/...">therefore navigates the frame, and any site sendingX-Frame-Options: deny/frame-ancestors 'none'— github.com, and most of the web — refuses to render there.target="_blank"doesn't help either: withoutallow-popupsthe sandbox blocks the popup, so those links do nothing at all.This has been true since the viewer shipped (
8c9b564, 2026-06-12): the sandbox has never carriedallow-popups, nothing ever rewrote link targets, and the overlay had no link handling.Fix
lib/docs/overlay.ts: intercept click / middle-click on anya[href]that resolves to a cross-document http(s) URL, and open it in a new tab from inside the user gesture. Same-document fragments (#section, the gutter section links) still navigate in-frame, non-http(s) schemes are left to the browser, and edit mode still treats links as text.sandbox/ CSPsandbox: addallow-popups allow-popups-to-escape-sandboxso that new tab is permitted and loads the linked site as a normal page rather than inheriting the opaque origin.The document still can't reach our origin, cookies, or storage — it gains only the ability to open a tab, which is strictly less than the in-frame navigation it already had.
Verification
X-Frame-Options: denytarget):chrome-error://chromewebdata/, no tab openswindow.openernull); middle-click behaves the same;#sectionlinks stay in-frame and open no tabnpm test(162 tests) andtsc --noEmitpass.Note
Medium Risk
Relaxes the viewer sandbox with allow-popups, but only for user-gesture tab opens; opaque-origin isolation for cookies/storage is unchanged.
Overview
Fixes external links in published docs replacing the viewer with “refused to connect” when the target blocks framing (e.g. GitHub). The sandboxed iframe and
/d/:slug/rawCSP now includeallow-popupsandallow-popups-to-escape-sandbox, and the injected overlay intercepts http(s) link clicks (and middle-click) towindow.open(..., "_blank", "noopener")instead of navigating the frame.Same-document
#fragmentlinks still navigate in-frame. The interceptor skips edit mode, overlay chrome (reaction chips, section link icons), and clicks that end with a non-empty text selection (comment/react). Highlight segment handlers ignore clicks the interceptor treated as navigation so links inside annotated spans still work.Reviewed by Cursor Bugbot for commit 99c12f4. Bugbot is set up for automated code reviews on this repo. Configure here.