Rework open paths: action routing, local PDF auto-open, file-access guide (+ PDF save)#5
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…le-access guide - Action click: blank new tab opens the empty viewer, non-PDF http(s) pages keep an in-page toast, uninjectable pages fall back to an OS notification; the "PDF" badge is removed. - Auto-open: DNR rule 3 intercepts local file:// PDFs (incl. Windows UNC) before commit; a webNavigation fallback routes to the viewer's file-access guide while the file-URL toggle is off; both paths respect the auto-open setting. - Viewer: file-access guide and missing-file states; missing-file only on MissingPDFException, settings page opens in the current tab. - Manifest: declarativeNetRequestWithHostAccess, webNavigation, notifications, file:///* host permission. - Docs: design doc (docs/issue-1-open-ux.md), implementation-plan §9 revision, Windows loading notes, progress log. Review follow-ups R1-R3 applied. Refs #1 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Standalone Windows 11 and macOS checklists (setup, expected results, V1/V2/V4 observation notes) in docs/issue-1-qa.md, linked from the design doc and progress log. Refs #1 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replacing Chrome's built-in viewer removed its download path; restore it in Margin. Saves the exact bytes being viewed via pdfDocument.getData() -> blob anchor, so it works offline and for drag-and-dropped files with no new permissions. Original basename kept (.pdf enforced), button disabled until a document loads, Ctrl/Cmd+S intercepts the browser save-page dialog. Also stop toolbar text buttons from wrapping per-character in narrow windows (nowrap + toolbar overflow-x scroll). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Design-only: maps QA feedback onto the planned-but-unbuilt pieces (mentions/links, manual crop, caption labels) plus new jump rules (1/8 alignment + flash) and card crop icon. Implementation targets a separate dev/figure-ux branch after issue #1 merges. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Body reference / hyperref / caption label clicks open the panel card without moving the text; the card jumps to the figure and mention chips jump to sentences, with the originating chip pinned as the return path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d9cd411ae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try { | ||
| const response = await fetch(url, { | ||
| method: 'HEAD', | ||
| method: 'GET', |
There was a problem hiding this comment.
When the current tab is a PDF served by a signed or one-time download URL such as /download?token=... that is not matched by isPdfLikeUrl, this probe performs a credentialed GET and then opens the viewer, where host.loadUrl fetches the same URL again. Aborting after headers does not prevent the server from marking the first GET as consumed, so the viewer can end up with a 403 or missing-PDF error for links that previously opened. Please avoid a separate consuming request, or reuse the fetched response, for this detection path.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
요약 (Refs #1)
이슈 #1에서 보고된 확장 동작 혼란(뱃지·자동 열기 무반응·Missing PDF)과 기능 요청(새탭 → 뷰어)을 통합 개편하고, 뷰어 PDF 저장 기능을 추가했다.
^file://.*\.pdf$로 파일 액세스 허용 시 커밋 전 무플래시 전환(Windows UNC 포함), 액세스 꺼짐 시 webNavigation 폴백이 뷰어의 권한 안내 화면으로 연결. 자동 열기 토글 OFF면 둘 다 비활성.MissingPDFException한정 — 손상 파일은 일반 오류).pdfDocument.getData()바이트를 blob으로 저장(재다운로드 없음, 신규 권한 없음). 내장 뷰어 대체로 사라졌던 저장 경로 복원.declarativeNetRequestWithHostAccess전환,webNavigation·notifications·file:///*추가.설계 문서:
docs/issue-1-open-ux.md(원인 분석 → 디자인 결정 → 스펙 → 태스크) / QA 가이드:docs/issue-1-qa.mdQA
docs/issue-1-qa.md의 W-1~W-33 체크리스트 확인 요청 (UNC 경로·탐색기 더블클릭·콜드 스타트 포함). 이슈 #1의 "Missing PDF" 재현 환경 검증이 특히 중요.머지 후 이슈 #1 답변·클로즈 예정.
🤖 Generated with Claude Code