fix(linux): correct browser webview boundaries and positioning using GTK overlays - #398
Conversation
📝 WalkthroughWalkthroughBrowser tab creation is asynchronous and performs Linux GTK overlay placement before recording tab state. Resize, visibility, and destruction update native child widgets. Renderer tests cover deterministic bounds updates and listener cleanup. ChangesBrowser webview placement
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BrowserTabCreate
participant BrowserTabManager
participant MainWebview
participant GTKOverlay
BrowserTabCreate->>BrowserTabManager: await create(tab_id, url, bounds)
BrowserTabManager->>MainWebview: dispatch GTK placement with with_webview
MainWebview->>GTKOverlay: add child widget and set bounds
GTKOverlay-->>BrowserTabManager: placement completes
BrowserTabManager-->>BrowserTabCreate: return BrowserTabInfo
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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.
Inline comments:
In `@src-tauri/src/browser_tab_manager.rs`:
- Around line 5-7: The blanket SendWrapper<T> unsafe impls are making all GTK
handles look thread-safe, but it is only used to pass main_widget into the
nested Linux with_webview path in BrowserTabManager::create. Remove
SendWrapper<T> and change the handoff so the GTK widget stays on the main
thread, or use a type-specific mechanism that does not require marking arbitrary
T as Send/Sync.
- Around line 230-297: The Linux GTK reparent path in
`BrowserTabManager::create` is swallowing failures by only logging inside
`handle_reparent()` and then continuing as if the tab was created successfully.
Update the `create()` flow so that a reparent failure from the
`main_webview`/`child_webview` setup is propagated back to the caller instead of
starting the poller, inserting the tab, and returning `Ok`. Use the existing
`handle_reparent` result and the surrounding `with_webview`/`create` logic to
return the error when reparenting fails.
- Around line 206-215: `BrowserBounds` is being converted twice on Linux by
dividing `bounds` by `scale_factor`, which breaks logical-pixel coordinates from
`getBoundingClientRect()`. Update the Linux handling in the browser tab bounds
path to keep `x`, `y`, `w`, and `h` in CSS/logical pixels end-to-end, and remove
the extra `scale_factor` division in this block. Also apply the same
logical-pixel treatment to the other Linux-specific bounds conversion paths in
`browser_tab_manager.rs` so all Tauri/GTK webview placement uses consistent
units.
In `@src/renderer/hooks/use-browser-webview.ts`:
- Line 1: The DOM-height diagnostics in use-browser-webview are too chatty
because updateBounds runs on ResizeObserver and visibility changes, causing
repeated console output and WARN IPCs before the webview exists. Update the
updateBounds flow in use-browser-webview to gate this logging to a one-shot or
dev-only path, and route any frontend error reporting through the existing
logFrontendError wrapper instead of direct console/IPC spam. Keep the fix
localized around the updateBounds logic and the webview creation/visibility
checks so split-layout resizes do not flood logs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 51d1d91e-bba0-4610-b2f7-68b7a6ada7b6
📒 Files selected for processing (2)
src-tauri/src/browser_tab_manager.rssrc/renderer/hooks/use-browser-webview.ts
3f9b0a2 to
482fdf6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src-tauri/src/commands.rs (1)
207-212: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winEnforce the attachment cap while reading.
The metadata check is point-in-time; a file that grows after the check is still read with
read_to_end, which can exceed the 10 MB cap before the post-read rejection. Read at mostATTACHMENT_MAX_IMAGE_BYTES + 1bytes before returning the result.Proposed fix
- file.read_to_end(&mut bytes) + file.take(ATTACHMENT_MAX_IMAGE_BYTES + 1) + .read_to_end(&mut bytes) .map_err(|e| format!("Failed to read attachment: {}", e))?;🤖 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-tauri/src/commands.rs` around lines 207 - 212, Update the attachment read flow around the existing file.read_to_end call to cap bytes read at ATTACHMENT_MAX_IMAGE_BYTES + 1, then retain the bytes.len() check so oversized attachments return the existing 10 MB limit error.
🤖 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.
Inline comments:
In `@src-tauri/src/browser_tab_manager.rs`:
- Around line 541-545: Update the Linux GTK branches of the generic show() and
hide() methods to propagate errors returned by with_webview, matching the
existing error-handling pattern in resize(). Remove the discarded-result
bindings and return or otherwise forward the dispatch failure so show() and
hide() cannot report success when the child widget update fails.
---
Outside diff comments:
In `@src-tauri/src/commands.rs`:
- Around line 207-212: Update the attachment read flow around the existing
file.read_to_end call to cap bytes read at ATTACHMENT_MAX_IMAGE_BYTES + 1, then
retain the bytes.len() check so oversized attachments return the existing 10 MB
limit error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 87207cd2-9cfb-4fdc-8193-1a3b46eb8bdd
📒 Files selected for processing (5)
src-tauri/Cargo.tomlsrc-tauri/src/browser_tab_manager.rssrc-tauri/src/commands.rssrc/renderer/hooks/use-browser-webview.test.tsxsrc/renderer/layouts/WorkspaceLayout.test.tsx
9cd97e3 to
ebcdb9a
Compare
Summary
Send/Syncwrapper around GTK handles.Related Issue
Closes #387
No duplicate pull request addressing this Linux GTK placement bug was found during the original contribution flow.
Type of Change
What Changed
BrowserTabManager::createasynchronous so GTK callback errors are returned through the existingIpcResultcontract.How It Was Tested
bun run ci(Biome lint + format + imports, strict mode)bun run typecheck(GitHub PR Validation: Lint, Typecheck & Test)bun run test(GitHub PR Validation: Lint, Typecheck & Test)cargo clippy --all-targets -- -D warnings(GitHub PR Validation: Rust Checks)cargo test(GitHub PR Validation: Rust Checks)Focused local validation completed:
npx vitest run src/renderer/hooks/use-browser-webview.test.tsxnpx biome check src/renderer/hooks/use-browser-webview.ts src/renderer/hooks/use-browser-webview.test.tsxcargo check --manifest-path src-tauri/Cargo.toml --all-targetsGitHub validation also passed the Windows Rust smoke check, standalone server build, web/Tauri frontend builds, CodeQL, dependency review, and secret scan.
Screenshots or Recordings
Existing reproduction screenshots are available in #387. The automated Linux Rust/build gates pass; no Linux desktop runtime was available for a new recording.
CI & Review Gate
Checklist