Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move tinyfiledialog call from script to embedder #23651

Closed
Closed
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Next

Move tinyfiledialog out of script into embedder.

  • Loading branch information
gatoWololo committed Jun 27, 2019
commit f356e40c351a1474ffddbbf4f3b384ad6b1bf3df

Some generated files are not rendered by default. Learn more.

@@ -154,6 +154,9 @@ pub enum EmbedderMsg {
GetSelectedBluetoothDevice(Vec<String>, IpcSender<Option<String>>),
/// Open file dialog to select files. Set boolean flag to true allows to select multiple files.
SelectFiles(Vec<FilterPattern>, bool, IpcSender<Option<Vec<String>>>),
/// Open yes/no message for user to allow permission specified by first String.
/// With dialog title specified by second String.
PromptPermission(String, String, IpcSender<PermissionRequest>),

This comment has been minimized.

@jdm

jdm Jun 28, 2019

Member

I propose instead of sending two strings, we send an enum:

enum PermissionName {
   // variants that match the DOM PermissionName enum
}

enum PermissionPrompt {
    Insecure(PermissionName),
    Request(PermissionName),
}

This gives embedders the information they require in order to display whatever native prompt interface would make sense on that platform and supports localization better in the future.

/// Request to present an IME to the user when an editable element is focused.
ShowIME(InputMethodType),
/// Request to hide the IME when the editable element is blurred.
@@ -186,6 +189,7 @@ impl Debug for EmbedderMsg {
EmbedderMsg::Panic(..) => write!(f, "Panic"),
EmbedderMsg::GetSelectedBluetoothDevice(..) => write!(f, "GetSelectedBluetoothDevice"),
EmbedderMsg::SelectFiles(..) => write!(f, "SelectFiles"),
EmbedderMsg::PromptPermission(..) => write!(f, "PromptPermission"),
EmbedderMsg::ShowIME(..) => write!(f, "ShowIME"),
EmbedderMsg::HideIME => write!(f, "HideIME"),
EmbedderMsg::Shutdown => write!(f, "Shutdown"),
@@ -200,3 +204,10 @@ impl Debug for EmbedderMsg {
/// the `String` content is expected to be extension (e.g, "doc", without the prefixing ".")
#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct FilterPattern(pub String);

// Status for prompting user for permission.

This comment has been minimized.

@jdm

jdm Jun 28, 2019

Member

Use /// here.

#[derive(Clone, Debug, Deserialize, Serialize)]
pub enum PermissionRequest {
Granted,
Denied,
}
@@ -28,9 +28,6 @@ phf_codegen = "0.7"
phf_shared = "0.7"
serde_json = "1.0"

[target.'cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))'.dependencies]
tinyfiledialogs = "3.0"

[dependencies]
app_units = "0.7"
backtrace = {version = "0.3", optional = true}
@@ -40,6 +40,7 @@ use crate::timers::{IsInterval, OneshotTimerCallback, OneshotTimerHandle};
use crate::timers::{OneshotTimers, TimerCallback};
use devtools_traits::{ScriptToDevtoolsControlMsg, WorkerId};
use dom_struct::dom_struct;
use embedder_traits::EmbedderMsg;
use ipc_channel::ipc::IpcSender;
use js::glue::{IsWrapper, UnwrapObjectDynamic};
use js::jsapi::JSObject;
@@ -55,7 +56,7 @@ use msg::constellation_msg::PipelineId;
use net_traits::image_cache::ImageCache;
use net_traits::{CoreResourceThread, IpcSend, ResourceThreads};
use profile_traits::{mem as profile_mem, time as profile_time};
use script_traits::{MsDuration, ScriptToConstellationChan, TimerEvent};
use script_traits::{MsDuration, ScriptMsg, ScriptToConstellationChan, TimerEvent};
use script_traits::{TimerEventId, TimerSchedulerMsg, TimerSource};
use servo_url::{MutableOrigin, ServoUrl};
use std::borrow::Cow;
@@ -378,6 +379,14 @@ impl GlobalScope {
&self.script_to_constellation_chan
}

pub fn send_to_embedder(&self, msg: EmbedderMsg) {
self.send_to_constellation(ScriptMsg::ForwardToEmbedder(msg));
}

pub fn send_to_constellation(&self, msg: ScriptMsg) {
self.script_to_constellation_chan().send(msg).unwrap();
}

pub fn scheduler_chan(&self) -> &IpcSender<TimerSchedulerMsg> {
&self.scheduler_chan
}
@@ -18,15 +18,14 @@ use crate::dom::globalscope::GlobalScope;
use crate::dom::permissionstatus::PermissionStatus;
use crate::dom::promise::Promise;
use dom_struct::dom_struct;
use embedder_traits::{EmbedderMsg, PermissionRequest};
use ipc_channel::ipc;
use js::conversions::ConversionResult;
use js::jsapi::{JSContext, JSObject};
use js::jsval::{ObjectValue, UndefinedValue};
use servo_config::pref;
use std::rc::Rc;
#[cfg(target_os = "linux")]
use tinyfiledialogs::{self, MessageBoxIcon, YesNo};

#[cfg(target_os = "linux")]
const DIALOG_TITLE: &'static str = "Permission request dialog";
const NONSECURE_DIALOG_MESSAGE: &'static str = "feature is only safe to use in secure context,\
but servo can't guarantee\n that the current context is secure. Do you want to proceed and grant permission?";
@@ -268,14 +267,10 @@ impl PermissionAlgorithm for Permissions {
PermissionState::Prompt => {
let perm_name = status.get_query();

let globalscope = GlobalScope::current().expect("No current global object");

// https://w3c.github.io/permissions/#request-permission-to-use (Step 3 - 4)
let state = prompt_user(
&format!("{} {} ?", REQUEST_DIALOG_MESSAGE, perm_name.clone()),
globalscope.is_headless(),
);

let globalscope = GlobalScope::current().expect("No current global object");
let prompt = format!("{} {} ?", REQUEST_DIALOG_MESSAGE, perm_name.clone());

This comment has been minimized.

@jdm

jdm Jun 28, 2019

Member

Rather than doing string formatting for dialogs in this code, I think we should delegate that to the embedder and only send an enum value that represents the kind of prompt required.

let state = prompt_user_from_embedder(prompt, &globalscope);
globalscope
.as_window()
.permission_state_invocation_results()
@@ -300,7 +295,7 @@ pub fn get_descriptor_permission_state(
env_settings_obj: Option<&GlobalScope>,
) -> PermissionState {
// Step 1.
let settings = match env_settings_obj {
let globalscope = match env_settings_obj {
Some(env_settings_obj) => DomRoot::from_ref(env_settings_obj),
None => GlobalScope::current().expect("No current global object"),
};
@@ -316,21 +311,19 @@ pub fn get_descriptor_permission_state(
if pref!(dom.permissions.testing.allowed_in_nonsecure_contexts) {
PermissionState::Granted
} else {
settings
globalscope
.as_window()
.permission_state_invocation_results()
.borrow_mut()
.remove(&permission_name.to_string());

prompt_user(
&format!("The {} {}", permission_name, NONSECURE_DIALOG_MESSAGE),
settings.is_headless(),
)
let prompt = format!("The {} {}", permission_name, NONSECURE_DIALOG_MESSAGE);
prompt_user_from_embedder(prompt, &globalscope)
}
};

// Step 3.
if let Some(prev_result) = settings
if let Some(prev_result) = globalscope
.as_window()
.permission_state_invocation_results()
.borrow()
@@ -340,7 +333,7 @@ pub fn get_descriptor_permission_state(
}

// Store the invocation result
settings
globalscope
.as_window()
.permission_state_invocation_results()
.borrow_mut()
@@ -350,28 +343,6 @@ pub fn get_descriptor_permission_state(
state
}

#[cfg(target_os = "linux")]
fn prompt_user(message: &str, headless: bool) -> PermissionState {
if headless {
return PermissionState::Denied;
}
match tinyfiledialogs::message_box_yes_no(
DIALOG_TITLE,
message,
MessageBoxIcon::Question,
YesNo::No,
) {
YesNo::Yes => PermissionState::Granted,
YesNo::No => PermissionState::Denied,
}
}

#[cfg(not(target_os = "linux"))]
fn prompt_user(_message: &str, _headless: bool) -> PermissionState {
// TODO popup only supported on linux
PermissionState::Denied
}

// https://w3c.github.io/permissions/#allowed-in-non-secure-contexts
fn allowed_in_nonsecure_contexts(permission_name: &PermissionName) -> bool {
match *permission_name {
@@ -399,3 +370,21 @@ fn allowed_in_nonsecure_contexts(permission_name: &PermissionName) -> bool {
PermissionName::Persistent_storage => false,
}
}

fn prompt_user_from_embedder(prompt: String, gs: &GlobalScope) -> PermissionState {
let (sender, receiver) = ipc::channel().expect("Failed to create IPC channel!");
gs.send_to_embedder(EmbedderMsg::PromptPermission(
prompt,
DIALOG_TITLE.to_string(),
sender,
));

match receiver.recv() {
Ok(PermissionRequest::Granted) => PermissionState::Granted,
Ok(PermissionRequest::Denied) => PermissionState::Denied,
Err(e) => {
warn!("Failed to receive permission state from embedder ({}).", e);
PermissionState::Denied
},
}
}
@@ -7,7 +7,7 @@ use crate::window_trait::{WindowPortsMethods, LINE_HEIGHT};
use euclid::{TypedPoint2D, TypedVector2D};
use keyboard_types::{Key, KeyboardEvent, Modifiers, ShortcutMatcher};
use servo::compositing::windowing::{WebRenderDebugOption, WindowEvent};
use servo::embedder_traits::{EmbedderMsg, FilterPattern};
use servo::embedder_traits::{EmbedderMsg, FilterPattern, PermissionRequest};
use servo::msg::constellation_msg::TopLevelBrowsingContextId as BrowserId;
use servo::msg::constellation_msg::TraversalDirection;
use servo::net_traits::pub_domains::is_reg_domain;
@@ -23,7 +23,7 @@ use std::mem;
use std::rc::Rc;
use std::thread;
use std::time::Duration;
use tinyfiledialogs::{self, MessageBoxIcon};
use tinyfiledialogs::{self, MessageBoxIcon, YesNo};

pub struct Browser<Window: WindowPortsMethods + ?Sized> {
current_url: Option<ServoUrl>,
@@ -401,6 +401,10 @@ where
self.event_queue.push(WindowEvent::SendError(None, reason));
};
},
EmbedderMsg::PromptPermission(message, dialog_title, sender) => {

This comment has been minimized.

@jdm

jdm Jun 28, 2019

Member

If you build ./mach build -d --libsimpleservo, you will see code in ports/libsimpleservo/api and ports/libsimpleservo/capi that need to handle this (by invoking a new method in HostTrait), and you can add stub implementations to ports/libsimpleservo/jniapi and ports/libmlservo for this new method as well.

let permission_state = prompt_user(&message, &dialog_title);
let _ = sender.send(permission_state);
}
EmbedderMsg::ShowIME(_kind) => {
debug!("ShowIME received");
},
@@ -419,6 +423,28 @@ where
}
}

#[cfg(target_os = "linux")]
fn prompt_user(prompt: &str, dialog_title: &str) -> PermissionRequest {
if opts::get().headless {
return PermissionRequest::Denied;
}
match tinyfiledialogs::message_box_yes_no(
dialog_title,
prompt,
MessageBoxIcon::Question,
YesNo::No,
) {
YesNo::Yes => PermissionRequest::Granted,
YesNo::No => PermissionRequest::Denied,
}
}

#[cfg(not(target_os = "linux"))]
fn prompt_user(_prompt: &str, _dialog_title: &str) -> PermissionRequest {
// TODO popup only supported on linux
PermissionRequest::Denied
}

#[cfg(target_os = "linux")]
fn platform_get_selected_devices(devices: Vec<String>) -> Option<String> {
let picker_name = "Choose a device";
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.