From 62f1dbebff443477376d9a8232726bb094906b5e Mon Sep 17 00:00:00 2001 From: chickenleaf Date: Tue, 4 Feb 2025 23:54:24 +0530 Subject: [PATCH] servoshell: Migrate to egui-file-dialog from tinyfiledialogs (#34823) This is the first step toward completely replacing tinyfiledialogs with an egui-based solution. Signed-off-by: L Ashwin B --- Cargo.lock | 119 +++++++++++++++++--- components/net/filemanager_thread.rs | 6 +- components/net/tests/filemanager_thread.rs | 2 +- components/script/dom/htmlinputelement.rs | 13 ++- components/shared/embedder/lib.rs | 3 +- components/shared/net/filemanager_thread.rs | 4 +- deny.toml | 6 + ports/servoshell/Cargo.toml | 1 + ports/servoshell/desktop/dialog.rs | 95 ++++++++++++++++ ports/servoshell/desktop/minibrowser.rs | 4 + ports/servoshell/desktop/mod.rs | 1 + ports/servoshell/desktop/webview.rs | 77 ++++++------- 12 files changed, 261 insertions(+), 70 deletions(-) create mode 100644 ports/servoshell/desktop/dialog.rs diff --git a/Cargo.lock b/Cargo.lock index f60b7c585c2..533dd65daac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1008,7 +1008,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "117725a109d387c937a1533ce01b450cbde6b88abceea8473c4d7a85853cda3c" dependencies = [ "lazy_static", - "windows-sys 0.48.0", + "windows-sys 0.59.0", ] [[package]] @@ -1600,6 +1600,15 @@ dependencies = [ "syn", ] +[[package]] +name = "directories" +version = "5.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a49173b84e034382284f27f1af4dcbbd231ffa358c0fe316541a7337f376a35" +dependencies = [ + "dirs-sys", +] + [[package]] name = "dirs" version = "5.0.1" @@ -1753,6 +1762,19 @@ dependencies = [ "profiling", ] +[[package]] +name = "egui-file-dialog" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac4167fcddb279f41863074b96b877c6e48c02d543b7d6111d1f3957b9a5874f" +dependencies = [ + "directories", + "dunce", + "egui", + "serde", + "sysinfo", +] + [[package]] name = "egui-winit" version = "0.30.0" @@ -1923,7 +1945,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "33d852cb9b869c2a9b3df2f71a3074817f01e1844f839a144f5fcef059a4eb5d" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -2431,7 +2453,7 @@ dependencies = [ "vec_map", "wasm-bindgen", "web-sys", - "windows", + "windows 0.58.0", ] [[package]] @@ -2450,7 +2472,7 @@ dependencies = [ "gobject-sys", "libc", "system-deps", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -2605,7 +2627,7 @@ dependencies = [ "log", "presser", "thiserror 1.0.69", - "windows", + "windows 0.58.0", ] [[package]] @@ -3891,7 +3913,7 @@ dependencies = [ "serde", "tempfile", "uuid", - "windows", + "windows 0.58.0", ] [[package]] @@ -3902,7 +3924,7 @@ checksum = "e19b23d53f35ce9f56aebc7d1bb4e6ac1e9c0db7ac85c8d1760c04379edced37" dependencies = [ "hermit-abi 0.4.0", "libc", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -4248,7 +4270,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fc2f4eb4bc735547cfed7c0a4922cbd04a4655978c09b54f1f7b228750664c34" dependencies = [ "cfg-if", - "windows-targets 0.48.5", + "windows-targets 0.52.6", ] [[package]] @@ -6128,7 +6150,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -6842,6 +6864,7 @@ dependencies = [ "cfg-if", "dirs", "egui", + "egui-file-dialog", "egui-winit", "egui_glow", "env_filter", @@ -7371,6 +7394,17 @@ dependencies = [ "syn", ] +[[package]] +name = "sysinfo" +version = "0.33.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4fc858248ea01b66f19d8e8a6d55f41deaf91e9d495246fd01368d99935c6c01" +dependencies = [ + "core-foundation-sys", + "libc", + "windows 0.57.0", +] + [[package]] name = "system-deps" version = "7.0.3" @@ -7431,7 +7465,7 @@ dependencies = [ "getrandom", "once_cell", "rustix", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -8680,7 +8714,7 @@ dependencies = [ "wasm-bindgen", "web-sys", "wgpu-types", - "windows", + "windows 0.58.0", "windows-core 0.58.0", ] @@ -8730,7 +8764,7 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.59.0", ] [[package]] @@ -8739,6 +8773,16 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows" +version = "0.57.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "12342cb4d8e3b046f3d80effd474a7a02447231330ef77d71daa6fbc40681143" +dependencies = [ + "windows-core 0.57.0", + "windows-targets 0.52.6", +] + [[package]] name = "windows" version = "0.58.0" @@ -8758,19 +8802,42 @@ dependencies = [ "windows-targets 0.52.6", ] +[[package]] +name = "windows-core" +version = "0.57.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2ed2439a290666cd67ecce2b0ffaad89c2a56b976b736e6ece670297897832d" +dependencies = [ + "windows-implement 0.57.0", + "windows-interface 0.57.0", + "windows-result 0.1.2", + "windows-targets 0.52.6", +] + [[package]] name = "windows-core" version = "0.58.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6ba6d44ec8c2591c134257ce647b7ea6b20335bf6379a27dac5f1641fcf59f99" dependencies = [ - "windows-implement", - "windows-interface", - "windows-result", + "windows-implement 0.58.0", + "windows-interface 0.58.0", + "windows-result 0.2.0", "windows-strings", "windows-targets 0.52.6", ] +[[package]] +name = "windows-implement" +version = "0.57.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9107ddc059d5b6fbfbffdfa7a7fe3e22a226def0b2608f72e9d552763d3e1ad7" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "windows-implement" version = "0.58.0" @@ -8782,6 +8849,17 @@ dependencies = [ "syn", ] +[[package]] +name = "windows-interface" +version = "0.57.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "29bee4b38ea3cde66011baa44dba677c432a78593e202392d1e9070cf2a7fca7" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "windows-interface" version = "0.58.0" @@ -8793,6 +8871,15 @@ dependencies = [ "syn", ] +[[package]] +name = "windows-result" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e383302e8ec8515204254685643de10811af0ed97ea37210dc26fb0032647f8" +dependencies = [ + "windows-targets 0.52.6", +] + [[package]] name = "windows-result" version = "0.2.0" @@ -8808,7 +8895,7 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4cd9b125c486025df0eabcb585e62173c6c9eddcec5d117d3b6e8c30e2ee4d10" dependencies = [ - "windows-result", + "windows-result 0.2.0", "windows-targets 0.52.6", ] diff --git a/components/net/filemanager_thread.rs b/components/net/filemanager_thread.rs index 007ebae78c4..418500ce7f3 100644 --- a/components/net/filemanager_thread.rs +++ b/components/net/filemanager_thread.rs @@ -582,7 +582,7 @@ impl FileManagerStore { patterns: Vec, multiple_files: bool, embedder_proxy: EmbedderProxy, - ) -> Option> { + ) -> Option> { let (ipc_sender, ipc_receiver) = ipc::channel().expect("Failed to create IPC channel!"); embedder_proxy.send(EmbedderMsg::SelectFiles( webview_id, @@ -605,7 +605,7 @@ impl FileManagerStore { patterns: Vec, sender: IpcSender>, origin: FileOrigin, - opt_test_path: Option, + opt_test_path: Option, embedder_proxy: EmbedderProxy, ) { // Check if the select_files preference is enabled @@ -636,7 +636,7 @@ impl FileManagerStore { patterns: Vec, sender: IpcSender>>, origin: FileOrigin, - opt_test_paths: Option>, + opt_test_paths: Option>, embedder_proxy: EmbedderProxy, ) { // Check if the select_files preference is enabled diff --git a/components/net/tests/filemanager_thread.rs b/components/net/tests/filemanager_thread.rs index 805f94d39ab..7a3789c59f9 100644 --- a/components/net/tests/filemanager_thread.rs +++ b/components/net/tests/filemanager_thread.rs @@ -49,7 +49,7 @@ fn test_filemanager() { patterns.clone(), tx, origin.clone(), - Some("tests/test.jpeg".to_string()), + Some("tests/test.jpeg".into()), )); let selected = rx .recv() diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index 5034e1ff787..bb9ab0f0f98 100644 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -6,7 +6,9 @@ use std::borrow::Cow; use std::cell::Cell; use std::cmp::Ordering; use std::ops::Range; +use std::path::PathBuf; use std::ptr::NonNull; +use std::str::FromStr; use std::{f64, ptr}; use dom_struct::dom_struct; @@ -1905,8 +1907,12 @@ impl HTMLInputElement { let target = self.upcast::(); if self.Multiple() { - let opt_test_paths = - opt_test_paths.map(|paths| paths.iter().map(|p| p.to_string()).collect()); + let opt_test_paths = opt_test_paths.map(|paths| { + paths + .iter() + .filter_map(|p| PathBuf::from_str(p).ok()) + .collect() + }); let (chan, recv) = ipc::channel(self.global().time_profiler_chan().clone()) .expect("Error initializing channel"); @@ -1930,7 +1936,7 @@ impl HTMLInputElement { if paths.is_empty() { return; } else { - Some(paths[0].to_string()) // neglect other paths + Some(PathBuf::from(paths[0].to_string())) // neglect other paths } }, None => None, @@ -2948,6 +2954,7 @@ impl Activatable for HTMLInputElement { fn filter_from_accept(s: &DOMString) -> Vec { let mut filter = vec![]; for p in split_commas(s) { + let p = p.trim(); if let Some('.') = p.chars().next() { filter.push(FilterPattern(p[1..].to_string())); } else if let Some(exts) = mime_guess::get_mime_extensions_str(p) { diff --git a/components/shared/embedder/lib.rs b/components/shared/embedder/lib.rs index 92e1ba9e6e6..dca4fa65abd 100644 --- a/components/shared/embedder/lib.rs +++ b/components/shared/embedder/lib.rs @@ -5,6 +5,7 @@ pub mod resources; use std::fmt::{Debug, Error, Formatter}; +use std::path::PathBuf; use base::id::{PipelineId, WebViewId}; use crossbeam_channel::Sender; @@ -213,7 +214,7 @@ pub enum EmbedderMsg { WebViewId, Vec, bool, - IpcSender>>, + IpcSender>>, ), /// Open interface to request permission specified by prompt. PromptPermission(WebViewId, PermissionPrompt, IpcSender), diff --git a/components/shared/net/filemanager_thread.rs b/components/shared/net/filemanager_thread.rs index 71fdfe50f0a..e55fbefcefc 100644 --- a/components/shared/net/filemanager_thread.rs +++ b/components/shared/net/filemanager_thread.rs @@ -141,7 +141,7 @@ pub enum FileManagerThreadMsg { Vec, IpcSender>, FileOrigin, - Option, + Option, ), /// Select multiple files. Last field is pre-selected file paths for testing @@ -150,7 +150,7 @@ pub enum FileManagerThreadMsg { Vec, IpcSender>>, FileOrigin, - Option>, + Option>, ), /// Read FileID-indexed file in chunks, optionally check URL validity based on boolean flag diff --git a/deny.toml b/deny.toml index f04b11d81b7..d1fe883bcc2 100644 --- a/deny.toml +++ b/deny.toml @@ -98,6 +98,12 @@ skip = [ "itertools", "toml", + # Duplicated by egui-file-dialog + "windows", + "windows-implement", + "windows-interface", + "windows-result", + # Duplicated by winit. "windows-sys", "windows-targets", diff --git a/ports/servoshell/Cargo.toml b/ports/servoshell/Cargo.toml index a97f04f5ba5..9ffdb60e7e4 100644 --- a/ports/servoshell/Cargo.toml +++ b/ports/servoshell/Cargo.toml @@ -122,6 +122,7 @@ serde_json = { workspace = true } shellwords = "1.0.0" surfman = { workspace = true, features = ["sm-x11", "sm-raw-window-handle-06"] } tinyfiledialogs = "3.0" +egui-file-dialog = "0.8.0" winit = "0.30.8" [target.'cfg(any(all(target_os = "linux", not(target_env = "ohos")), target_os = "windows"))'.dependencies] diff --git a/ports/servoshell/desktop/dialog.rs b/ports/servoshell/desktop/dialog.rs new file mode 100644 index 00000000000..650c65d22d7 --- /dev/null +++ b/ports/servoshell/desktop/dialog.rs @@ -0,0 +1,95 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use egui_file_dialog::{DialogState, FileDialog as EguiFileDialog}; +use log::warn; +use servo::ipc_channel::ipc::IpcSender; +use servo::FilterPattern; + +#[derive(Debug)] +pub struct FileDialog { + dialog: EguiFileDialog, + multiple: bool, + response_sender: IpcSender>>, +} + +#[derive(Debug)] +pub enum Dialog { + File(FileDialog), +} + +impl Dialog { + pub fn new_file_dialog( + multiple: bool, + response_sender: IpcSender>>, + patterns: Vec, + ) -> Self { + let mut dialog = EguiFileDialog::new(); + if !patterns.is_empty() { + dialog = dialog + .add_file_filter( + "All Supported Types", + Arc::new(move |path: &Path| { + path.extension() + .and_then(|e| e.to_str()) + .map_or(false, |ext| { + let ext = ext.to_lowercase(); + patterns.iter().any(|pattern| ext == pattern.0) + }) + }), + ) + .default_file_filter("All Supported Types"); + } + + let dialog = FileDialog { + dialog, + multiple, + response_sender, + }; + + Dialog::File(dialog) + } + + pub fn update(&mut self, ctx: &egui::Context) -> bool { + match self { + Dialog::File(dialog) => { + if dialog.dialog.state() == DialogState::Closed { + if dialog.multiple { + dialog.dialog.pick_multiple(); + } else { + dialog.dialog.pick_file(); + } + } + + let state = dialog.dialog.update(ctx).state(); + + match state { + DialogState::Open => true, + DialogState::Selected(path) => { + if let Err(e) = dialog.response_sender.send(Some(vec![path])) { + warn!("Failed to send file selection response: {}", e); + } + false + }, + DialogState::SelectedMultiple(paths) => { + if let Err(e) = dialog.response_sender.send(Some(paths)) { + warn!("Failed to send file selection response: {}", e); + } + false + }, + DialogState::Cancelled => { + if let Err(e) = dialog.response_sender.send(None) { + warn!("Failed to send cancellation response: {}", e); + } + false + }, + DialogState::Closed => false, + } + }, + } + } +} diff --git a/ports/servoshell/desktop/minibrowser.rs b/ports/servoshell/desktop/minibrowser.rs index d30dacde8fb..eb0173b9aef 100644 --- a/ports/servoshell/desktop/minibrowser.rs +++ b/ports/servoshell/desktop/minibrowser.rs @@ -391,6 +391,10 @@ impl Minibrowser { return; }; + egui::CentralPanel::default().show(ctx, |_| { + webview.update(ctx); + }); + CentralPanel::default() .frame(Frame::none()) .show(ctx, |ui| { diff --git a/ports/servoshell/desktop/mod.rs b/ports/servoshell/desktop/mod.rs index 44c6204b785..cf612e89272 100644 --- a/ports/servoshell/desktop/mod.rs +++ b/ports/servoshell/desktop/mod.rs @@ -6,6 +6,7 @@ pub(crate) mod app; pub(crate) mod cli; +mod dialog; mod egui_glue; mod embedder; pub(crate) mod events_loop; diff --git a/ports/servoshell/desktop/webview.rs b/ports/servoshell/desktop/webview.rs index ce6a646d95f..2d6a7e57d87 100644 --- a/ports/servoshell/desktop/webview.rs +++ b/ports/servoshell/desktop/webview.rs @@ -5,6 +5,7 @@ use std::collections::HashMap; use std::fs::File; use std::io::Write; +use std::path::PathBuf; use std::rc::Rc; use std::{env, thread}; @@ -29,6 +30,7 @@ use servo::{ }; use tinyfiledialogs::{self, MessageBoxIcon, OkCancel, YesNo}; +use super::dialog::Dialog; use super::keyutils::CMD_OR_CONTROL; use super::window_trait::{WindowPortsMethods, LINE_HEIGHT}; use crate::desktop::tracing::trace_embedder_msg; @@ -62,6 +64,7 @@ pub struct WebView { pub focused: bool, pub load_status: LoadStatus, pub servo_webview: ::servo::WebView, + dialogs: Vec, } impl WebView { @@ -73,8 +76,23 @@ impl WebView { focused: false, load_status: LoadStatus::Complete, servo_webview, + dialogs: vec![], } } + + pub fn add_file_dialog( + &mut self, + multiple: bool, + response_sender: IpcSender>>, + patterns: Vec, + ) { + self.dialogs + .push(Dialog::new_file_dialog(multiple, response_sender, patterns)); + } + + pub fn update(&mut self, ctx: &egui::Context) { + self.dialogs.retain_mut(|dialog| dialog.update(ctx)); + } } pub struct ServoEventResponse { @@ -173,6 +191,15 @@ impl WebViewManager { self.status_text.clone() } + pub fn has_pending_file_dialog(&self) -> bool { + self.focused_webview().map_or(false, |webview| { + webview + .dialogs + .iter() + .any(|dialog| matches!(dialog, Dialog::File(_))) + }) + } + // Returns the webviews in the creation order. pub fn webviews(&self) -> Vec<(WebViewId, &WebView)> { let mut res = vec![]; @@ -417,7 +444,8 @@ impl WebViewManager { opts: &Opts, messages: Vec, ) -> ServoEventResponse { - let mut need_present = self.load_status() != LoadStatus::Complete; + let mut need_present = + self.load_status() != LoadStatus::Complete || self.has_pending_file_dialog(); let mut need_update = false; for message in messages { trace_embedder_msg!(message, "{message:?}"); @@ -662,16 +690,10 @@ impl WebViewManager { }; }, EmbedderMsg::SelectFiles(webview_id, patterns, multiple_files, sender) => { - let result = match (opts.headless, get_selected_files(patterns, multiple_files)) - { - (true, _) | (false, None) => sender.send(None), - (false, Some(files)) => sender.send(Some(files)), - }; - if let Err(e) = result { - self.send_error( - webview_id, - format!("Failed to send SelectFiles response: {e}"), - ); + if let Some(webview) = self.get_mut(webview_id) { + webview.add_file_dialog(multiple_files, sender, patterns); + need_update = true; + need_present = true; }; }, EmbedderMsg::PromptPermission(_, prompt, sender) => { @@ -863,39 +885,6 @@ fn platform_get_selected_devices(devices: Vec) -> Option { None } -fn get_selected_files(patterns: Vec, multiple_files: bool) -> Option> { - let picker_name = if multiple_files { - "Pick files" - } else { - "Pick a file" - }; - thread::Builder::new() - .name("FilePicker".to_owned()) - .spawn(move || { - let mut filters = vec![]; - for p in patterns { - let s = "*.".to_string() + &p.0; - filters.push(tiny_dialog_escape(&s)) - } - let filter_ref = &(filters.iter().map(|s| s.as_str()).collect::>()[..]); - let filter_opt = if !filters.is_empty() { - Some((filter_ref, "")) - } else { - None - }; - - if multiple_files { - tinyfiledialogs::open_file_dialog_multi(picker_name, "", filter_opt) - } else { - let file = tinyfiledialogs::open_file_dialog(picker_name, "", filter_opt); - file.map(|x| vec![x]) - } - }) - .unwrap() - .join() - .expect("Thread spawning failed") -} - // This is a mitigation for #25498, not a verified solution. // There may be codepaths in tinyfiledialog.c that this is // inadquate against, as it passes the string via shell to