Skip to content

fix(accessibility): keep read-only link shortcut inert - #152

Draft
seonghobae wants to merge 3 commits into
mainfrom
fix/read-only-link-shortcut
Draft

fix(accessibility): keep read-only link shortcut inert#152
seonghobae wants to merge 3 commits into
mainfrom
fix/read-only-link-shortcut

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Problem

CwlEditor documents editable={false} as read-only with no editing actions and hides its editing toolbar in that state, but the shared EditorFrame keyboard handler still processed Mod+K. A read-only editor could therefore open the link-editing prompt and attempt an editing command.

RED -> fix -> GREEN

  • Protected base and current live protected tip: main@50ac98cfa0ad9e8dd75f93ca437a5679fed4d804.
  • Test-only RED exact head 084d739437d2db4070ee8547b265316dd4c9e420 was rerun in CI run 31419836520, build/test job 93558861504, which explicitly checked out that SHA. Typecheck passed; coverage failed only on CwlEditor.accessibility.test.tsx because Ctrl+K invoked window.prompt("Link URL", "https://") once. Result: 140 test files / 818 tests passed, with the one new read-only shortcut regression failing as intended.
  • Narrow source fix 547ba5e68a55efce7bfd5dded313cd5a7b909fc6 gates the existing shared keyboard handler on the existing editable authority and adds editable to the callback dependency set. It does not add a second state source or change editable-mode shortcut behavior.
  • CodeRabbit reviewed that predecessor head and correctly found the regression test covered Ctrl+K but not the parallel Meta+K path used on macOS.
  • Current exact head ac419f48fc0894a1d61e4a48dd36015440df4a9b adds the Meta+K assertion while preserving the Ctrl+K assertion.

Exact-head proof

For unchanged exact head ac419f48fc0894a1d61e4a48dd36015440df4a9b:

  • CI run 31420426964: success;
  • Security Scan run 31420426951: success;
  • SAST Semgrep run 31420426965: success;
  • CI passed typecheck, exact 100% owned-production coverage, build, packed-package consumer verification, demo build, cross-engine browser evidence, and Office Python 3.11–3.14 gates;
  • fresh review-thread inspection shows no unresolved inline threads;
  • the predecessor CodeRabbit finding is addressed by the current test; and
  • a current-head CodeRabbit re-review was explicitly requested, but the service reported its review limit had been reached and identified this exact delta (547ba5e... -> ac419f48...) as deferred until the quota resets. That rate limit is not treated as passing review evidence.

Automated/model status is not qualifying independent formal approval. Any subsequent head movement invalidates this proof and must be revalidated.

Integration dependency

This remains Draft because protected main@50ac98cfa0ad9e8dd75f93ca437a5679fed4d804 is still the exact 0.6.0 stable release candidate tracked by #118. Do not advance protected main until that registry operational-acceptance boundary is resolved. Revalidate live main, exact head, review state, and live policy before any eventual readiness or merge transition.

Summary by CodeRabbit

  • 버그 수정

    • 읽기 전용 편집기에서 Ctrl+K 단축키 사용 시 편집 관련 대화상자가 표시되지 않도록 수정했습니다.
    • 편집할 수 없는 상태에서 키보드 입력이 처리되지 않도록 개선했습니다.
  • 테스트

    • 읽기 전용 상태의 키보드 단축키 동작에 대한 접근성 회귀 테스트를 추가했습니다.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

EditorFrame이 읽기 전용 상태에서 키보드 이벤트를 처리하지 않도록 변경했습니다. CwlEditor 접근성 테스트는 Ctrl+K 입력 시 window.prompt가 호출되지 않는지 검증합니다.

Changes

읽기 전용 링크 단축키

Layer / File(s) Summary
키보드 이벤트 가드 및 회귀 테스트
src/components/EditorFrame.tsx, src/components/CwlEditor.accessibility.test.tsx
editable이 false이면 키보드 이벤트 처리를 중단합니다. onKeyDown 의존성 배열이 최신 editable 값을 사용합니다. 테스트는 DOM 정리와 Vitest mock 복원을 수행하며, 읽기 전용 상태의 Ctrl+K에서 window.prompt가 호출되지 않는지 확인합니다.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 읽기 전용 편집기에서 링크 단축키를 비활성화하는 주요 변경 사항을 정확하고 간결하게 설명합니다.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/read-only-link-shortcut

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/components/CwlEditor.accessibility.test.tsx (1)

79-95: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Meta+K 경로도 회귀 테스트에 포함하세요.

EditorFrameevent.metaKey || event.ctrlKey를 처리하지만, 현재 테스트는 ctrlKey만 설정합니다. macOS에서 Meta+K 경로만 회귀해도 이 테스트는 통과합니다. metaKey: true도 동일한 검증에 포함하세요.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/CwlEditor.accessibility.test.tsx` around lines 79 - 95, Update
the read-only shortcut test around CwlEditor to cover the Meta+K path as well as
Ctrl+K: trigger the keydown with metaKey enabled and verify window.prompt
remains uncalled, while preserving the existing read-only behavior assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/components/CwlEditor.accessibility.test.tsx`:
- Around line 79-95: Update the read-only shortcut test around CwlEditor to
cover the Meta+K path as well as Ctrl+K: trigger the keydown with metaKey
enabled and verify window.prompt remains uncalled, while preserving the existing
read-only behavior assertion.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e5670188-abbe-415a-ab57-4989d784d018

📥 Commits

Reviewing files that changed from the base of the PR and between 50ac98c and 547ba5e.

📒 Files selected for processing (2)
  • src/components/CwlEditor.accessibility.test.tsx
  • src/components/EditorFrame.tsx

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant