fix: keep mobile review handoff popup-safe (#143)

frontend/index.html

@@ -87,6 +87,8 @@ textarea { resize: vertical; min-height: 120px; }
 .review-feedback select { min-height:44px; padding:8px; border-radius:8px; border:1px solid #1f3a5f; background:#0b1526; color:#e5e7eb; }
 .review-handoff { position:sticky; bottom:0; z-index:3; display:grid; gap:8px; padding:10px 4px; background:rgba(11,21,38,.98); border-top:1px solid #2a496e; }
 .review-handoff button { min-height:44px; }
+.review-handoff-link { min-height:44px; display:flex; align-items:center; justify-content:center; border:1px solid #60a5fa; border-radius:8px; color:#bfdbfe; font-weight:700; text-decoration:none; }
+.review-handoff-link[hidden] { display:none; }
 .review-copy-fallback { min-height:140px; }
 .review-progress-actions { position:sticky; bottom:0; z-index:2; display:flex; align-items:center; justify-content:space-between; gap:10px; margin:8px -4px 0; padding:10px 4px; background:rgba(11,21,38,.96); border-top:1px solid #2a496e; }
 .review-progress-actions button { min-height:44px; max-width:100%; }
@@ -264,6 +266,7 @@ textarea { resize: vertical; min-height: 120px; }
         <button id="copy-review-feedback">Copy feedback &amp; continue in Gitea</button>
         <span class="small" id="review-handoff-status" aria-live="polite"></span>
         <textarea class="review-copy-fallback" id="review-copy-fallback" readonly hidden aria-label="Feedback to copy manually"></textarea>
+        <a class="review-handoff-link" id="review-handoff-link" target="_blank" rel="noopener noreferrer" hidden>Continue to Gitea</a>
       </div>
     </section>
     <h2>Review history</h2>
@@ -323,6 +326,7 @@ textarea { resize: vertical; min-height: 120px; }
   let reviewFiles = [];
   let bulkConfirmationPending = false;
   let bulkMarkPending = false;
+  let reviewHandoffPending = false;
 
   async function fetchReviewJson(url, options) {
     const response = await fetch(url, options);
@@ -533,6 +537,8 @@ textarea { resize: vertical; min-height: 120px; }
     qs('#review-summary').value = '';
     qs('#review-copy-fallback').hidden = true;
     qs('#review-copy-fallback').value = '';
+    qs('#review-handoff-link').hidden = true;
+    qs('#review-handoff-link').href = item.url;
     qs('#review-handoff-status').textContent = '';
     draft = null;
     reviewFiles = [];
@@ -735,24 +741,43 @@ textarea { resize: vertical; min-height: 120px; }
   qs('#review-summary').addEventListener('input', event => draft?.setSummary(event.target.value));
   qs('#review-decision').addEventListener('change', event => draft?.setDecision(event.target.value));
   qs('#copy-review-feedback').addEventListener('click', async () => {
-    if (!draft || !selectedReview) return;
+    if (!draft || !selectedReview || reviewHandoffPending) return;
+    reviewHandoffPending = true;
+    qs('#copy-review-feedback').disabled = true;
     const fallback = qs('#review-copy-fallback');
+    const directLink = qs('#review-handoff-link');
     fallback.hidden = true;
-    const copied = await createReviewController.copyAndContinue({
-      text: createReviewController.formatFeedback(draft.snapshot(), reviewFiles),
-      url: selectedReview.url,
-      copy: text => navigator.clipboard.writeText(text),
-      open: url => window.open(url, '_blank', 'noopener,noreferrer'),
-      fallback: text => {
-        fallback.value = text;
-        fallback.hidden = false;
-        fallback.focus();
-        fallback.select();
-      },
-    });
-    qs('#review-handoff-status').textContent = copied ?
-      'Feedback copied. Opening Gitea…' :
-      'Clipboard unavailable. Copy the selected feedback, then use Open in Gitea.';
+    directLink.hidden = true;
+    let handoffWindow = null;
+    try {
+      const result = await createReviewController.copyAndContinue({
+        text: createReviewController.formatFeedback(draft.snapshot(), reviewFiles),
+        url: selectedReview.url,
+        copy: text => navigator.clipboard.writeText(text),
+        open: () => {
+          handoffWindow = window.open('about:blank', '_blank');
+          if (handoffWindow) handoffWindow.opener = null;
+          return handoffWindow;
+        },
+        fallback: text => {
+          fallback.value = text;
+          fallback.hidden = false;
+          fallback.focus();
+          fallback.select();
+        },
+      });
+      if (result.opened) {
+        qs('#review-handoff-status').textContent = 'Feedback copied. Gitea opened in a new tab.';
+      } else {
+        directLink.hidden = false;
+        qs('#review-handoff-status').textContent = result.copied ?
+          'Feedback copied. Your browser blocked the new tab; continue with the link below.' :
+          'Clipboard unavailable. Copy the selected feedback, then continue to Gitea.';
+      }
+    } finally {
+      reviewHandoffPending = false;
+      qs('#copy-review-feedback').disabled = false;
+    }
   });
 
   const contextPoller = createContextPoller({

frontend/review-sheet.js

@@ -166,14 +166,20 @@ function formatFeedback(draft, files) {
 }
 
 async function copyAndContinue({ text, url, copy, open, fallback }) {
+  const destination = open();
   try {
     await copy(text);
   } catch (error) {
+    destination?.close?.();
     fallback(text);
-    return false;
+    return { copied: false, opened: false };
   }
-  open(url);
-  return true;
+  if (!destination) {
+    fallback(text);
+    return { copied: true, opened: false };
+  }
+  destination.location.href = url;
+  return { copied: true, opened: true };
 }
 
 createReviewController.renderDiffFile = renderDiffFile;

tests/test_my_work.py

@@ -534,25 +534,29 @@ process.stdout.write(markdown);
     )
 
 
-def test_review_handoff_opens_gitea_only_after_copy_and_preserves_failed_copy():
+def test_review_handoff_reserves_window_before_copy_and_reports_blocked_popup():
     script = f"""
 const reviewSheet = require({json.dumps(str(REVIEW_SHEET))});
 const events = [];
+let finishCopy;
+const reserved = {{ location: {{ href: '' }}, close: () => events.push('close') }};
 const success = reviewSheet.copyAndContinue({{
   text: 'feedback', url: 'https://forge.example/pulls/7',
-  copy: async text => events.push('copy:' + text),
-  open: url => events.push('open:' + url),
+  copy: text => new Promise(resolve => {{ events.push('copy:' + text); finishCopy = resolve; }}),
+  open: () => {{ events.push('reserve'); return reserved; }},
   fallback: text => events.push('fallback:' + text),
 }});
-const failedEvents = [];
-const failure = reviewSheet.copyAndContinue({{
-  text: 'keep me', url: 'https://forge.example/pulls/8',
-  copy: async () => {{ throw new Error('denied'); }},
-  open: url => failedEvents.push('open:' + url),
-  fallback: text => failedEvents.push('fallback:' + text),
+const beforeCopySettles = events.slice();
+finishCopy();
+const blockedEvents = [];
+const blocked = reviewSheet.copyAndContinue({{
+  text: 'copied', url: 'https://forge.example/pulls/8',
+  copy: async () => blockedEvents.push('copy'),
+  open: () => {{ blockedEvents.push('reserve'); return null; }},
+  fallback: text => blockedEvents.push('fallback:' + text),
 }});
-Promise.all([success, failure]).then(results => process.stdout.write(JSON.stringify({{
-  events, failedEvents, results
+Promise.all([success, blocked]).then(results => process.stdout.write(JSON.stringify({{
+  beforeCopySettles, events, blockedEvents, destination: reserved.location.href, results
 }})));
 """
     result = subprocess.run(
@@ -560,9 +564,35 @@ Promise.all([success, failure]).then(results => process.stdout.write(JSON.string
     )
 
     assert json.loads(result.stdout) == {
-        "events": ["copy:feedback", "open:https://forge.example/pulls/7"],
-        "failedEvents": ["fallback:keep me"],
-        "results": [True, False],
+        "beforeCopySettles": ["reserve", "copy:feedback"],
+        "events": ["reserve", "copy:feedback"],
+        "blockedEvents": ["reserve", "copy", "fallback:copied"],
+        "destination": "https://forge.example/pulls/7",
+        "results": [
+            {"copied": True, "opened": True},
+            {"copied": True, "opened": False},
+        ],
+    }
+
+
+def test_review_handoff_closes_reserved_window_when_copy_fails():
+    script = f"""
+const reviewSheet = require({json.dumps(str(REVIEW_SHEET))});
+const events = [];
+reviewSheet.copyAndContinue({{
+  text: 'keep me', url: 'https://forge.example/pulls/8',
+  copy: async () => {{ throw new Error('denied'); }},
+  open: () => ({{ close: () => events.push('close') }}),
+  fallback: text => events.push('fallback:' + text),
+}}).then(result => process.stdout.write(JSON.stringify({{ events, result }})));
+"""
+    result = subprocess.run(
+        ["node", "-e", script], check=True, capture_output=True, text=True
+    )
+
+    assert json.loads(result.stdout) == {
+        "events": ["close", "fallback:keep me"],
+        "result": {"copied": False, "opened": False},
     }
 
 
@@ -611,6 +641,9 @@ async def test_mobile_review_sheet_captures_and_safely_hands_off_feedback():
     assert 'id="review-summary"' in html
     assert 'id="copy-review-feedback"' in html
     assert 'id="review-copy-fallback"' in html
+    assert 'id="review-handoff-link"' in html
+    assert '.review-handoff-link' in html and 'min-height:44px' in html
+    assert '.review-handoff-link[hidden]' in html and 'display:none' in html
     assert '.review-note' in html and 'min-height:88px' in html
     assert '.review-handoff' in html and 'position:sticky' in html
     assert "createReviewController.createDraft" in html
@@ -620,7 +653,8 @@ async def test_mobile_review_sheet_captures_and_safely_hands_off_feedback():
     assert "createReviewController.formatFeedback" in html
     assert "createReviewController.copyAndContinue" in html
     assert "navigator.clipboard.writeText" in html
-    assert "window.open(url, '_blank', 'noopener,noreferrer')" in html
+    assert "window.open('about:blank', '_blank')" in html
+    assert "handoffWindow.opener = null" in html
 
 
 @pytest.mark.anyio