Skip to content

M4: drag bookmark trees and insert after selection (#2120) - #2238

Open
robertobulltech wants to merge 2 commits into
storytold:mainfrom
robertobulltech:feat/bookmark-tree-drag-drop
Open

robertobulltech wants to merge 2 commits into
storytold:mainfrom
robertobulltech:feat/bookmark-tree-drag-drop

Conversation

@robertobulltech

@robertobulltech robertobulltech commented Oct 14, 2026 •

Copy link
Copy Markdown

Main integration — 2026-10-17

Integrated upstream main at 924c4da4 in c9f19a4e, preserving the existing feature scope and branch history.

Local validation passed: formatting; strict all-target Clippy on affected crates; relevant library/integration tests; layering, asset attribution and parity gates; CLI without default features; dependency licence/advisory checks; and all 31 library crates on WASM with the pinned craft-fonts input. Native builds/tests use Rust 1.99, the low-disk profile, two build jobs and isolated Cargo targets. Existing ignored hardware/measurement probes remain explicit in the logs.

Full-workspace Linux/Windows CI for this exact commit: https://github.com/robertobulltech/pdfcraft/actions/runs/39020745613. The run is still in progress at publication; workflow dispatch also starts the additional nightly mutation-fuzz job.

The earlier validation history and feature limitations below remain for context.


What this changes

Fixes #2120. The Bookmarks sidebar accepts before/after/last-child drops, moves complete subtrees across parents and back to the root, scrolls near its edges, and expands a collapsed destination after 600 ms. Invalid, unchanged and cancelled drops perform no edit; a successful drop performs exactly one existing Edit::MoveBookmark, with undo/redo.

New Bookmark (toolbar and Ctrl/Cmd+B) inserts immediately after the selected bookmark at the same hierarchy level, targeting the currently viewed page. It selects the new bookmark, reveals its ancestors and focuses the existing inline rename field. With no selection, the existing root insertion behavior remains.

bookmark_ui.rs holds document-local selection, rename, expansion and drag state. Drop indexes account for source removal; destination parent paths remain in original coordinates because the engine adjusts them. Successful edits remap selection and expansion paths; revision changes, tab/panel changes, permissions, search, Escape and invalid/outside releases cancel stale drags. Undo/reload/external edits clear stale selection rather than infer identity from duplicate titles. PDF outline writing and metadata preservation remain in the existing engine/organize operations. Existing bookmark_add and bookmark_move automation tools provide headless access.

How you verified it

Linux, Rust 1.99; existing target cache, --profile low-disk -j 2 for native builds/tests/Clippy.

  • cargo fmt --all -- --check
  • cargo clippy --profile low-disk -p pdfcraft-ui-egui -p pdfcraft-engine -p pdfcraft-organize -j 2 --all-targets -- -D warnings
  • cargo test --profile low-disk -p pdfcraft-ui-egui -j 2 --lib --test bookmarks --test viewer --test control --test commands: 375 library tests passed / 1 existing measurement ignored; bookmarks 22, commands 23, control 56, viewer 36 passed. Final bookmark rerun after fixture improvements: 22 passed.
  • cargo test --profile low-disk -p pdfcraft-organize -p pdfcraft-engine -j 2 --lib bookmark: organize 10 and engine 4 passed
  • cargo test --profile low-disk -p pdfcraft-organize -j 2 --lib: 119 passed / 1 existing external Poppler oracle ignored
  • cargo test --profile low-disk -p pdfcraft-automation -j 2 --test automation bookmark: 5 passed
  • cargo build --profile low-disk -p pdfcraft -p pdfcraft-cli -j 2
  • cargo xtask layers, assets, parity (through cargo run --profile low-disk -p xtask -j 2 -- <gate>); parity retains its existing note about 96 shipped entries without tools, unrelated to bookmark add/move
  • cargo check -p pdfcraft-ui-egui -j 2 --target wasm32-unknown-unknown, with a temporary matching Rust sysroot: passed, seven warnings in unchanged code. The first system-toolchain attempt failed because the WASM standard library was absent (E0463, missing core). Extracted the matching distribution target package locally, verified its SHA-256 against the package database, and retried with RUSTFLAGS=--sysroot=<temporary sysroot>. No system compiler or repository dependency changes.

The real-pointer sibling drag regression and selected-root insertion regression were run against restored, unmodified upstream production sources: both failed with unchanged bookmark order / root append, then passed with this implementation. An independent remove/insert model checks every root gap and every selection across eight siblings; related-subtree cases check destination-parent shifting and descendant selection.

Runtime-generated fixtures cover empty/root/nested trees, multiple siblings, three hierarchy levels, duplicate titles with distinct destinations, multiline titles, collapsed/expanded subtrees, a truncated depth-limited tree, and 120 bookmarks. Tests exercise actual primary-pointer drags, read-only permissions, source/descendant/no-op rejection, Escape/outside/filter/panel/tab/close/mutation cancellation, nested insertion and rename focus, selection remapping, undo/redo, and exact preorder/depth/page comparison after save/close/reopen. No binary fixture is committed.

Held dragging on the 120-bookmark fixture took approximately 41 ms for 80 headless frames (~0.52 ms/frame), excluding native GPU rendering; pointer movement left the PDF unchanged and a distant drop succeeded. This is a small synthetic observation, not a large-file performance guarantee.

Built Linux application verification used an isolated portable copy under Xvfb with --control and PDFCRAFT_SYSTEM_FONTS=0: actual ui.drag moved D before A; ui.command bookmark.add focused an Untitled editor and inserted a named sibling after D on page 2. Inspected the real UI screenshot, saved, reopened through automation, and checked the order/destinations. CLI run --script --root also exercised the existing bookmark tools and page_render; both saved synthetic outputs passed pdfcraft-cli check.

Never-crash, provenance and security

  • No new production panic shortcuts, unsafe code, dependencies, assets, fonts, workers or network behavior
  • Checked path access and index arithmetic; outline traversal retains existing display bounds
  • No Adobe assets or proprietary/GPL/AGPL implementation consulted
  • Existing engine permissions, signed-document rules, transactional edits and save validation remain authoritative
  • MCP and UI control remain opt-in; control token was not logged or exposed
  • Parity evidence and milestone/progress notes updated; contributor files unchanged by request

Limitations / reviewer focus

Draft for independent review. Reordering is explicitly disabled while bookmark search is active or the sidebar reports an incomplete outline (its existing 10,000-item / 33-level limit), so hidden children cannot corrupt insertion calculations. The headless bookmark_move tool remains available for those trees. The existing sidebar size/depth limits and nonvirtual row layout are retained.

Review the original-parent/post-removal-index distinction and path remapping, especially when moving into a later sibling's descendants. Selection and temporary expansion state are cleared after undo/reload/untracked mutations and other edits whose outline effects the UI does not track; selecting a bookmark again restores an insertion reference.

Native runtime verification was Linux under Xvfb, not physical Windows/macOS/FreeBSD or browser testing. WASM compilation passed. Full workspace tests and remote CI have not been claimed.

Based on upstream main 9931329b; #2120 remained open and no equivalent open PR/upstream branch/recent implementation was found on the final recheck.

Co-Authored-By: Codex (GPT-6) <noreply@openai.com>
@robertobulltech
robertobulltech marked this pull request as ready for review October 14, 2026 22:23

This branch has not been deployed

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

Labels

None yet

Projects

None yet

1 participant