feat(browser): browser hardening bundle — find-in-page, downloads, zoom, security indicator#150
Merged
Merged
Conversation
Rebasing feat/browser-hardening-bundle onto updated main revealed that from_state was dropped from BrowserView (the bundle simplified new but forgot from_state). browser_pane.rs::new_from_state calls it for tab state restore on pane reopen. Adds from_state mirroring new: uses BrowserModel::restore, builds webviews from all persisted tabs, wires url_editor + find_editor subscriptions and event_rx the same way. 84 browser tests pass (downloads, zoom, popup_policy, url_input, persistence).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR lands parts of the “browser hardening bundle” into warp-app’s embedded browser pane, adding download destination resolution and per-tab zoom support, and refactoring native webview hosting and browser view behavior to match the updated bundle integration (including restoring BrowserView::from_state construction logic).
Changes:
- Added a new
browser::downloadsmodule and wired WKWebView download-start/completed handlers to save into the user’s Downloads folder with filename sanitization and collision suffixing. - Added per-tab zoom controls (Zoom in/out/reset) with Chrome-like discrete zoom steps and overflow-menu actions.
- Refactored browser view/webview hosting code (including loading-strip rendering changes and reinstating a
from_stateconstructor path).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| app/src/browser/webview_host.rs | Wires download handlers, adds set_zoom, and adjusts native attach / bounds / visibility behavior. |
| app/src/browser/mod.rs | Updates module exports to include the new downloads module (and removes several prior browser hardening modules from the module tree). |
| app/src/browser/downloads.rs | New pure helper module for download directory selection, filename derivation/sanitization, collision suffixing, and log-safe URL formatting (with unit tests). |
| app/src/browser/browser_view.rs | Adds zoom state/actions and updates UI behavior (tab strip sizing token alignment, loading strip rendering changes, state restoration constructor changes). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
250
to
255
| let mut builder = wry::WebViewBuilder::new_as_child(&parent) | ||
| .with_url(url) | ||
| .with_bounds(Self::wry_rect(bounds)) | ||
| .with_visible(self.desired_visible) | ||
| .with_accept_first_mouse(true) | ||
| .with_devtools(devtools_enabled) | ||
| // Install the alert/confirm/prompt shim before any page | ||
| // script runs. wry 0.38 has no native JS-dialog handler | ||
| // API on macOS, so pages that call these can otherwise | ||
| // hang. See `dialogs.rs` for the full reasoning. | ||
| .with_initialization_script(dialogs::INIT_SCRIPT) | ||
| // Deny camera / microphone / geolocation / notification | ||
| // permission requests at the JS layer. wry 0.38 hardcodes | ||
| // `WKPermissionDecisionGrant` on macOS with no override | ||
| // hook, so this is the only reachable defense without | ||
| // forking wry. See `permissions.rs`. | ||
| .with_initialization_script(permissions::INIT_SCRIPT) | ||
| // Make the `castcodes://` scheme actually load something | ||
| // instead of erroring out as "scheme not supported". Our | ||
| // URL normalizer already passes it through to the | ||
| // webview; the handler here defines the route table. | ||
| .with_custom_protocol(warp_core::brand::PUBLIC_URL_SCHEME.to_string(), |request| { | ||
| castcodes_protocol::handle(&request) | ||
| }) | ||
| .with_document_title_changed_handler(move |title| { |
| .with_url(url) | ||
| .with_bounds(Self::wry_rect(bounds)) | ||
| .with_visible(self.desired_visible) | ||
| .with_accept_first_mouse(true) |
Comment on lines
201
to
212
| #[cfg(not(target_family = "wasm"))] | ||
| if let Some(webview) = &self.webview { | ||
| let rect = Self::wry_rect(bounds); | ||
| let rect_key = (rect.x, rect.y, rect.width, rect.height); | ||
|
|
||
| if self.last_sent_rect != Some(rect_key) { | ||
| if let Err(err) = webview.set_bounds(rect) { | ||
| log::warn!("failed to resize browser pane webview: {err}"); | ||
| } else { | ||
| self.last_sent_rect = Some(rect_key); | ||
| } | ||
| if let Err(err) = webview.set_bounds(rect) { | ||
| log::warn!("failed to resize browser pane webview: {err}"); | ||
| } | ||
| if self.desired_visible { | ||
| if self.last_sent_visible == Some(true) { | ||
| return; | ||
| } | ||
| if let Err(err) = webview.set_visible(true) { | ||
| log::warn!("failed to show browser pane webview: {err}"); | ||
| } else { | ||
| self.last_sent_visible = Some(true); | ||
| } | ||
| } |
|
|
||
| #[cfg(target_family = "wasm")] | ||
| #[cfg(not(target_os = "macos"))] | ||
| let _ = (window_id, bounds, app); |
Comment on lines
+143
to
+145
| let next = (current as usize).saturating_add(1); | ||
| let max = (ZOOM_STEPS.len() - 1) as u8; | ||
| (next as u8).min(max) |
Comment on lines
684
to
687
| self.sync_active_tab_into_editor(ctx); | ||
| self.sync_pane_title(ctx); | ||
| self.schedule_persist(ctx); | ||
| ctx.notify(); | ||
| } |
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
Lands the complete browser hardening bundle onto main. Squash of PRs #126, #136, #137 plus review follow-ups and integration fixes.
Changes
feat(browser): popup handler, page-load events, security indicator (PR #126)
Popup policy, page-load spinner wiring, SSL/security indicator in URL bar.
feat(browser): find-in-page overlay
Cmd+F find overlay; BrowserViewAction::ToggleFind/FindNext/FindPrev/CloseFind; JS injected via webview_host.
feat(browser): accessible tab-close tooltip
feat(browser): align TAB_STRIP_HEIGHT to design token 34px (Action 18)
feat(browser): wire download handler to ~/Downloads w/ collision suffix (PR #136 — Action 10)
WKWebView downloads route to ~/Downloads; safe filename sanitization, percent-decoded URL fallback, collision suffix (report.pdf → report (1).pdf). Gated to macOS. 11 unit tests in browser::downloads.
feat(browser): zoom controls (PR #137 — Action 16)
Per-tab zoom via overflow menu (Zoom in/out/Reset). Fixed zoom_step_in u8 OOB wrap and active_tab_zoom_level type mismatch. 7 unit tests.
fix(browser): restore from_state constructor
from_state was dropped during bundle simplification but browser_pane::new_from_state calls it for tab-state restore on pane reopen.
Verification
cargo check -p warp-app— clean (10 pre-existing warnings only)cargo test -p warp-app --lib 'browser::'