feat(browser): browser hardening bundle v2 — find-in-page, downloads, zoom, design token#149
Merged
Conversation
…curity indicator Lands six follow-up actions from the browser pane audit: - **Action 2**: URL bar now treats `javascript:` (case-insensitive) as a search query instead of executing the script. Mirrors Chrome/Safari/Firefox defense against pasted-bookmarklet self-XSS. - **Action 1**: Per-pane `wry::WebContext` rooted at `<warp_home_config_dir>/browser/data`. Wired correctly so it activates on Linux/webkit2gtk and whenever wry adds macOS `WKWebsiteDataStore` plumbing; today wry 0.38 ignores it on macOS (see `data_dir.rs` for the reality check). WKWebView's default data store is already per-app-sandbox so cookies were never shared with Safari — the original audit overstated this risk. - **Action 5**: `window.open` / `target="_blank"` requests now route through `popup_policy::decide`. http/https/file/about/castcodes → new in-pane tab; mailto/tel/sms and unknown schemes → system handler; javascript:/data: popups → blocked. The handler always returns `false` to wry so no OS-level window is ever spawned. - **Action 7**: `with_navigation_handler` + `with_on_page_load_handler` drive the loading flag end-to-end. The previous heuristic (clear loading when document.title arrives) left pages with no/static titles stuck "loading" forever. Navigation events also resync the model URL on HTTP redirects. - **Action 11**: 2px accent-colored progress strip below the toolbar, always rendered (no reflow) and only painted while the active tab is loading. - **Action 13**: Closed-padlock icon in the URL bar for HTTPS (and loopback http); amber alert-triangle for plaintext http to a remote host. New `Icon::LockClosed` enum variant + matching SVG asset (the existing `Icon::Lock` ships an unlocked padlock, wrong semantic). The host-side surface changes: - `webview_host.rs`: introduces `NativeWebViewEvent` (TitleChanged, LoadingChanged, NavigationStarted, PopupOpenTab, PopupOpenExternal), replacing the title-only channel. `NativeBrowserWebView::new` now takes a `Option<SharedWebContext>`. - `browser_model.rs`: `set_title` no longer clears `loading` as a side effect — adds `set_loading_for(tab_id, bool)` and `replace_current_url_for(tab_id, url)` that don't disturb history (used for redirect resync). - `browser_view.rs`: holds one `SharedWebContext` per pane (shared by every tab), dispatches popup-tab requests via a new `open_tab` helper reused by the New Tab action. 40 unit tests pass (added 11 new — popup_policy classifier coverage and javascript-URL handling). No new clippy lints. Action 19 (tab-close a11y tooltip) was pulled from this bundle: `Hoverable` has no tooltip API and switching to `Button` would change the close button's 16×16 sizing to the icon-button standard 24×24. Filed as follow-up needing a hoverable-tooltip wrapper or framework support. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The SSL/security indicator helper added in c7a7182 was untested. Adds 5 cases: - https → Secure (case-insensitive scheme match) - remote http → Insecure (with and without port) - loopback http (localhost / 127.0.0.1 / ::1 / 0.0.0.0, with and without port + path) → Secure - about:/file:/data:/castcodes: → Neutral - empty input → Neutral These would have caught a regression where (e.g.) a case-only-uppercase scheme bypassed the secure classification. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Adds the compact icon button helper for the browser tab close control, scopes close clicks to the child button, and keeps the helper docs aligned with its compact styling.
Adds the browser find-in-page overlay using injected JavaScript plus IPC, layered on the browser hardening stack.
…ix (Action 10) Wires WKWebView/Wry download handlers to route files into ~/Downloads with safe, collision-resistant filenames. Adds browser::downloads module with sanitization, URL-based fallback naming, percent-decoding, and unit tests. gated to macOS. All review threads resolved.
Adds per-tab zoom controls in the overflow menu (Zoom in/out/Reset). Tracks zoom step per TabId, applies on tab switch and after pane restore. Fixes zoom_step_in OOB u8 wrap and active_tab_zoom_level type mismatch. All review threads resolved.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR merges the remaining “browser hardening bundle v2” work into app/src/browser/, adding user-facing browser-pane features (find-in-page, downloads, zoom) and additional hardening/UX improvements (popup policy routing, page-load lifecycle + loading strip, security indicator, and tab-close accessibility), plus a new lock icon asset.
Changes:
- Add a find-in-page overlay (host-side model + injected JS + IPC results channel).
- Add deterministic macOS download destination handling with filename sanitization and collision suffixing.
- Add per-tab zoom state and UI actions (overflow menu), plus URL-bar security indicator and page-load loading strip.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
crates/warp_core/src/ui/icons.rs |
Adds Icon::LockClosed and maps it to a new bundled SVG asset for the security indicator. |
app/src/ui_components/buttons.rs |
Introduces small_icon_button_with_color for compact icon buttons (used for tab close with tooltip). |
app/src/browser/webview_host.rs |
Expands webview → host event channel (title/load/nav/popup/find results), wires download handlers, popup policy, IPC parsing, and zoom script hooks. |
app/src/browser/url_input.rs |
Rejects javascript: in the address bar by rewriting it to a search query; adds tests. |
app/src/browser/popup_policy.rs |
New pure popup URL classifier (Tab / External / Block) with unit tests. |
app/src/browser/mod.rs |
Exposes new browser submodules (data_dir, downloads, find, popup_policy). |
app/src/browser/find.rs |
New find overlay state model + JS injection helpers + IPC message shape + unit tests. |
app/src/browser/downloads.rs |
New download destination resolution logic (sanitization, suffixing, log-safe URL) with unit tests. |
app/src/browser/data_dir.rs |
New per-pane web data dir resolver for wry::WebContext (with platform notes). |
app/src/browser/browser_view.rs |
Main UI integration: security indicator, loading strip, find overlay, zoom actions/state, updated close-tab button tooltip, event handling, and tests. |
app/src/browser/browser_model.rs |
Splits title vs loading updates and adds redirect URL replacement API used by navigation events. |
app/assets/bundled/svg/lock-closed-01.svg |
Adds the closed-lock SVG asset. |
app/assets/bundled/js/find.js |
Adds injected find-in-page implementation posting results back via window.ipc. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot stopped work on behalf of
BunsDev due to an error
May 25, 2026 17:59
- zoom_step_in: use u8::saturating_add + min(max) to avoid usize
cast truncation at boundary (e.g. current=255 no longer wraps to 0)
- webview_host IPC handler: reject payloads above 8 KB before JSON
parse to prevent expensive allocations from arbitrary page messages
- test imports: rustfmt-format the use super::{...} list in tests module
All 81 browser unit tests pass.
- browser_view.rs: keep saturating_add fix for zoom_step_in (HEAD)
- browser_view.rs: keep from_state method from main (new feature)
- browser_view.rs: keep formatted use super::{...} imports (HEAD)
- webview_host.rs: keep 8 KB IPC payload size guard (HEAD)
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
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.
Summary
Merges the remaining browser hardening bundle commits into main, covering Actions 10, 16, 17, and 18 plus review follow-ups.
What's included
feat(browser): hardening bundle — popup handler, page-load events, security indicator
Wires popup policy (block/allow), page-load events for spinner state, and SSL/security indicator in the URL bar.
test(browser): cover classify_security branches
Unit tests for all SecurityState variants.
fix(browser): address PR 126 review findings
Review-requested improvements from the initial hardening bundle PR.
feat(browser): add accessible tab-close tooltip
aria-label-style tooltip on tab close button for accessibility.feat(browser): add find-in-page overlay
Cmd+F opens a find overlay; find_next/prev/clear/toggle wired through BrowserViewAction and webview_host script injection.
feat(browser): align TAB_STRIP_HEIGHT to design token 34px (Action 18)
Locks the constant to the
--tabbar-heightCSS design token with a guard test.feat(browser): wire download handler to ~/Downloads w/ collision suffix (Action 10)
Routes WKWebView downloads to
~/Downloadswith safe filename sanitization, percent-decoded URL fallback, and collision suffix (report.pdf→report (1).pdf). Gated to macOS. Unit tests inbrowser::downloads.feat(browser): zoom controls (Action 16)
Per-tab zoom state tracked in
BrowserView::tab_zoom_steps. Overflow menu exposes Zoom in/out/Reset with a dynamic label. Fixedzoom_step_inu8 OOB wrap andactive_tab_zoom_leveltype mismatch. 7 unit tests.Verification
cargo test -p warp-app --lib browser::browser_view::tests::zoom→ 7/7 passcargo test -p warp-app --lib browser::downloads→ all pass./script/check_rebrandand./script/check_ai_attributionpass