Persistent per-page comments (view/create split, tags, threads, import/export) - #306
Open
craighagan wants to merge 9 commits into
Open
Persistent per-page comments (view/create split, tags, threads, import/export)#306craighagan wants to merge 9 commits into
craighagan wants to merge 9 commits into
Conversation
Select text and click the 💬 tooltip or press ⌘⇧K to add comments. Comments are stored in chrome.storage.local and can be exported to a .comments.json file via the sidebar download button. - content/comments.js: selection handling, comment input UI, highlights, sidebar panel, text anchoring, export trigger - content/comments.css: theme-aware styling with dark mode and responsive - background/comments.js: context menu, chrome.storage.local CRUD, export via downloads API - manifest.chrome.json: +contextMenus, +downloads permissions, v5.3.1 - DESIGN-comments.md: design doc with architecture and review outcomes
Adds a 'comments' toggle in the popup Content tab (defaults to on). When disabled, comments scripts and CSS are not injected. Includes storage migration for existing installs.
Resolve All marks all comments as resolved in one click. Delete All prompts for confirmation then clears all comments.
Two export options in sidebar: - ⬇ JSON: exports .comments.json (structured data) - ⬇ MD: exports .commented.md with HTML comments injected inline after each anchored text selection Markdown export finds each comment's anchor text in the source and inserts <!-- COMMENT: body --> immediately after it. Resolved comments are tagged <!-- COMMENT [RESOLVED]: body -->.
Click Edit on any comment to turn the body into an editable textarea. Save with ⌘Enter or the Save button. Cancel with Escape.
Phase 1 — Metadata & Filtering: - Author name (from chrome.storage.sync 'commentsAuthor') - Comment tags: note, question, suggestion, issue, outdated, action-needed - Severity levels: low, medium, high, critical (colored pills) - Filter sidebar by status, tag, severity - Search across comment text, anchors, tags, authors - Keyboard nav: Ctrl+] next, Ctrl+[ previous comment Phase 2 — Editing Power: - Suggestion mode: propose replacement text with diff display - Import comments from .comments.json file (merge or replace) - Search box in sidebar with real-time filtering - Auto-linkify URLs in comment body Phase 3 — Collaboration & Polish: - Threaded replies on each comment - Extension icon badge showing unresolved count per tab - Click anywhere on comment card to scroll to highlighted text - Orphan handling: unanchored comments show warning pill + heading hint - Improved button tooltips for clarity - Roadmap document with collaboration strategy
- Parse <!-- COMMENT: ... --> tags from markdown source on page load (fetches raw file via XHR if <pre> element is gone) - Imported inline comments merge with stored comments and become editable - Click anywhere in document closes sidebar (configurable via commentsCloseOnClick) - Orphan detection: comments whose anchor text is missing show⚠️ pill + heading hint - Fixed scroll-to-highlight reliability (replaced scrollIntoView with absolute positioning) - Fixed sidebar header button wrapping (nowrap, tighter padding) - Keyboard nav: Ctrl+] next, Ctrl+[ previous comment - Severity-colored highlight borders on anchored text
…n Mithril Adversarial review of the comments feature (SDE/SysDE/security) found blocking issues: XSS via unescaped tag/severity/id in innerHTML, silent data loss on MV3 service worker restart, and comments silently going unanchored when their anchor text spans an inline element boundary. A second review round after remediation found further issues (dead fallback code path, comments UI getting destroyed by the main app's Mithril redraw, stale controlled-textarea state). - Split content.comments (view: highlights + sidebar) from content.commentsWrite (create: tooltip, cmd+shift+K, context menu, all mutating actions). New installs default to view-only; existing users who already had comments enabled keep write access via migration. - Rewrite all comment-content rendering (sidebar, comment items, tooltip, input/edit/reply boxes) as Mithril components instead of innerHTML template strings, eliminating the escaping-surface class of bug by construction. Add sanitizeComment/sanitizeTag/ sanitizeSeverity/sanitizeId allowlist validation on every entry point (storage load, inline-comment parse, JSON import, fresh creation). - Guard against treating a failed/asleep-service-worker sendMessage response as "zero stored comments," which previously risked overwriting real storage with an inline-only merge. - Fix cross-element highlight anchoring: highlightAcrossNodes() is now actually reachable and wraps per-text-node segments when a comment's anchor spans an inline element (bold/code/link). - Mount the comments UI and toggle button on document.documentElement instead of document.body, since content/index.js's own body-level Mithril mount fully owns and diffs body's children and was destroying both on every theme/autoreload/raw-view redraw. - Add a debounced MutationObserver to reapply highlights after the main app's Mithril re-render replaces the html/markdown container wholesale. - Fix stale controlled-textarea state in the edit/reply boxes and strip draft scratch fields before persisting. - Add a confirmation prompt to Resolve All, matching Delete All. - Clarify popup labels: comments/commentsWrite now display as 'Show comments' / 'Create comments' instead of raw storage keys. - Bump manifest to 5.3.2.
- Add COMMENTS-README.md documenting the shipped comments feature: the comments/commentsWrite read-write settings split, usage, storage/merge behavior, cross-element anchoring, autoreload highlight recovery, import/export, data model, and keyboard shortcuts. Link it from README.md's Content Options table. - Remove DESIGN-comments.md and ROADMAP-comments.md from version control. Both describe an earlier, abandoned design (native messaging host + sidecar JSON file on disk) and a stale roadmap that no longer match the shipped chrome.storage.local-based implementation. Kept locally for reference, now gitignored. - Bump manifest to 5.4, matching the project's established two-part version scheme (v2.7 through v5.3) rather than continuing the 5.3.1/5.3.2 three-part departure introduced with this feature.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds persistent, per-page comments for rendered markdown: select any text to attach a comment, anchored and highlighted inline, with a sidebar for browsing/filtering/searching.
What's included
Cmd/Ctrl+Shift+K→ comment input. Sidebar lists all comments for the page, filterable by status/tag/severity, with search.note/question/suggestion/issue/outdated/action-neededtag andlow/medium/high/criticalseverity per comment, rendered as pills.<!-- COMMENT: ... -->markers — a round-trip mechanism for sharing comments via the document itself (e.g. committing them to a repo) instead of separate storage. Import reads a previously exported JSON file back in.chrome.storage.local, keyed by page URL. No new permissions beyond what the extension already needs for local file access.comments(defaulttrue) — view-only: highlights + sidebar browsing.commentsWrite(defaultfalse) — adds everything needed to author comments (tooltip, shortcut, context menu, all mutating sidebar actions). Has no effect withoutcommentsalso on. New installs default to view-only.COMMENTS-README.md, plus a Content Options table entry inREADME.md.5.4.Why the read/write split
Auto-highlighting arbitrary local files and letting the extension append/inject comment UI everywhere felt like it warranted a more conservative default than "on and fully interactive everywhere," especially for a viewer that's frequently pointed at other people's files. Splitting view from create lets someone browse comments left by a teammate (via the exported/imported JSON or inline markers) without the extension also being able to mutate anything, and keeps the interactive surface (selection listeners, keyboard shortcuts, context menu) opt-in.
Implementation notes for review
m()), matching the rest of the extension's rendering approach — noinnerHTMLstring-building for anything that includes comment text, tags, or IDs.[\w-]+, and text fields are length-capped.<code>or<a>) is highlighted by wrapping each affected text node individually, rather than requiring the whole match to sit in one DOM node.MutationObserver.document.documentElement(a sibling of<body>) rather than as a child of<body>, since the existing app'sm.mount($('body'), ...)fully owns and diffsbody's children on every redraw.Testing
node --check.build/package.sh chromebuilds cleanly.Cmd/Ctrl+Shift+K,Ctrl+]/Ctrl+[,Cmd/Ctrl+Enter,Escape), highlight survival across autoreload/theme/raw-view toggles, and the read-only vs. write-enabled setting combinations.