From 5cd3a5368103973974b867be654ecc5430bc1f0a Mon Sep 17 00:00:00 2001 From: Ryan McGrath Date: Sat, 29 Feb 2020 15:34:07 -0800 Subject: [PATCH] Warnings/imports/unused code cleanup, rework some enums that seemed a bit wonky in hindsight, set WKWebView download delegate pieces as a feature flag since it's technically a private API --- .gitignore | 2 ++ appkit/Cargo.toml | 3 ++ appkit/src/app/events.rs | 16 ++-------- appkit/src/app/mod.rs | 3 +- appkit/src/events.rs | 54 ++++++++++++++++++++------------ appkit/src/file_panel/save.rs | 5 ++- appkit/src/file_panel/select.rs | 4 +-- appkit/src/file_panel/traits.rs | 8 ++--- appkit/src/menu/item.rs | 26 ++++++++++----- appkit/src/menu/menu.rs | 2 +- appkit/src/networking/mod.rs | 2 +- appkit/src/view/class.rs | 2 +- appkit/src/webview/action.rs | 10 +++--- appkit/src/webview/config.rs | 4 +-- appkit/src/webview/controller.rs | 48 ++++++++++++++-------------- appkit/src/webview/mod.rs | 2 +- appkit/src/webview/traits.rs | 42 +++++++++++++++++++++---- appkit/src/webview/webview.rs | 4 +-- appkit/src/window/controller.rs | 6 ++-- appkit/src/window/window.rs | 15 +++++++-- 20 files changed, 155 insertions(+), 103 deletions(-) diff --git a/.gitignore b/.gitignore index a9d37c5..7ea5e83 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,4 @@ target Cargo.lock +placeholder +placeholder2 diff --git a/appkit/Cargo.toml b/appkit/Cargo.toml index b2edcf6..af6a006 100644 --- a/appkit/Cargo.toml +++ b/appkit/Cargo.toml @@ -16,3 +16,6 @@ lazy_static = "1" objc = "0.2.7" objc_id = "0.1.1" uuid = { version = "0.8", features = ["v4"] } + +[features] +enable-webview-downloading = [] diff --git a/appkit/src/app/events.rs b/appkit/src/app/events.rs index a6a43ea..502e8aa 100644 --- a/appkit/src/app/events.rs +++ b/appkit/src/app/events.rs @@ -2,25 +2,13 @@ //! //! Now, I know what you're thinking: this is dumb. //! -//! And sure, maybe. But if you've ever opened Xcode and wondered why the hell -//! you have a xib/nib in your macOS project, it's (partly) because *that* handles -//! the NSMenu architecture for you... an architecture that, supposedly, is one of the -//! last Carbon pieces still laying around. -//! -//! And I gotta be honest, I ain't about the xib/nib life. SwiftUI will hopefully clear -//! that mess up one day, but in the meantime, we'll do this. -//! -//! Now, what we're *actually* doing here is relatively plain - on certain key events, -//! we want to make sure cut/copy/paste/etc are sent down the event chain. Usually, the -//! xib/nib stuff handles this for you... but this'll mostly do the same. +//! However, there are rare cases where this is beneficial, and by default we're doing nothing... +//! so consider this a placeholder that we might use in the future for certain things. use std::sync::Once; -use cocoa::base::{id, nil}; - use objc::declare::ClassDecl; use objc::runtime::Class; -use objc::{class, msg_send, sel, sel_impl}; /// Used for injecting a custom NSApplication. Currently does nothing. pub(crate) fn register_app_class() -> *const Class { diff --git a/appkit/src/app/mod.rs b/appkit/src/app/mod.rs index 8ebf406..d3840df 100644 --- a/appkit/src/app/mod.rs +++ b/appkit/src/app/mod.rs @@ -4,7 +4,6 @@ use std::sync::Once; use cocoa::base::{id, nil}; -use cocoa::foundation::NSString; use cocoa::appkit::{NSRunningApplication}; use objc_id::Id; @@ -25,7 +24,7 @@ pub trait AppDelegate { fn did_finish_launching(&self) {} fn did_become_active(&self) {} - fn on_message(&self, message: Self::Message) {} + fn on_message(&self, _message: Self::Message) {} } /// A wrapper for `NSApplication`. It holds (retains) pointers for the Objective-C runtime, diff --git a/appkit/src/events.rs b/appkit/src/events.rs index 27fa0c0..cb114f9 100644 --- a/appkit/src/events.rs +++ b/appkit/src/events.rs @@ -1,27 +1,41 @@ //! Hoists some type definitions in a way that I personally find cleaner than what's in the Servo //! code. -#[allow(non_upper_case_globals, non_snake_case)] -pub mod NSEventModifierFlag { - use cocoa::foundation::NSUInteger; +use cocoa::foundation::NSUInteger; - /// Indicates the Caps Lock key has been pressed. - pub const CapsLock: NSUInteger = 1 << 16; - - /// Indicates the Control key has been pressed. - pub const Control: NSUInteger = 1 << 18; - - /// Indicates the Option key has been pressed. - pub const Option: NSUInteger = 1 << 19; - - /// Indicates the Command key has been pressed. - pub const Command: NSUInteger = 1 << 20; - - /// Indicates device-independent modifier flags are in play. - pub const DeviceIndependentFlagsMask: NSUInteger = 0xffff0000; +#[derive(Clone, Copy, Debug)] +pub enum EventModifierFlag { + CapsLock, + Control, + Option, + Command, + DeviceIndependentFlagsMask } -#[allow(non_upper_case_globals, non_snake_case)] -mod NSEventType { - pub const KeyDown: usize = 10; +impl From for NSUInteger { + fn from(flag: EventModifierFlag) -> NSUInteger { + match flag { + EventModifierFlag::CapsLock => 1 << 16, + EventModifierFlag::Control => 1 << 18, + EventModifierFlag::Option => 1 << 19, + EventModifierFlag::Command => 1 << 20, + EventModifierFlag::DeviceIndependentFlagsMask => 0xffff0000 + } + } +} + +impl From<&EventModifierFlag> for NSUInteger { + fn from(flag: &EventModifierFlag) -> NSUInteger { + match flag { + EventModifierFlag::CapsLock => 1 << 16, + EventModifierFlag::Control => 1 << 18, + EventModifierFlag::Option => 1 << 19, + EventModifierFlag::Command => 1 << 20, + EventModifierFlag::DeviceIndependentFlagsMask => 0xffff0000 + } + } +} + +pub enum EventType { + KeyDown } diff --git a/appkit/src/file_panel/save.rs b/appkit/src/file_panel/save.rs index 6466f26..88da1f9 100644 --- a/appkit/src/file_panel/save.rs +++ b/appkit/src/file_panel/save.rs @@ -4,14 +4,13 @@ use block::ConcreteBlock; -use cocoa::base::{id, nil, YES, NO, BOOL}; -use cocoa::foundation::{NSInteger, NSUInteger, NSString}; +use cocoa::base::{id, nil, YES, NO}; +use cocoa::foundation::{NSInteger, NSString}; use objc::{class, msg_send, sel, sel_impl}; use objc::runtime::Object; use objc_id::ShareId; -use crate::file_panel::enums::ModalResponse; use crate::utils::str_from; #[derive(Debug)] diff --git a/appkit/src/file_panel/select.rs b/appkit/src/file_panel/select.rs index 92700bb..2397b82 100644 --- a/appkit/src/file_panel/select.rs +++ b/appkit/src/file_panel/select.rs @@ -4,8 +4,8 @@ use block::ConcreteBlock; -use cocoa::base::{id, nil, YES, NO, BOOL}; -use cocoa::foundation::{NSInteger, NSUInteger}; +use cocoa::base::{id, YES, NO}; +use cocoa::foundation::NSInteger; use objc::{class, msg_send, sel, sel_impl}; use objc::runtime::Object; diff --git a/appkit/src/file_panel/traits.rs b/appkit/src/file_panel/traits.rs index a080497..d8e4e00 100644 --- a/appkit/src/file_panel/traits.rs +++ b/appkit/src/file_panel/traits.rs @@ -4,18 +4,18 @@ pub trait OpenSaveController { /// Called when the user has entered a filename (typically, during saving). `confirmed` /// indicates whether or not they hit the save button. - fn user_entered_filename(&self, filename: &str, confirmed: bool) {} + fn user_entered_filename(&self, _filename: &str, _confirmed: bool) {} /// Notifies you that the panel selection changed. fn panel_selection_did_change(&self) {} /// Notifies you that the user changed directories. - fn did_change_to_directory(&self, url: &str) {} + fn did_change_to_directory(&self, _url: &str) {} /// Notifies you that the Save panel is about to expand or collapse because the user /// clicked the disclosure triangle that displays or hides the file browser. - fn will_expand(&self, expanding: bool) {} + fn will_expand(&self, _expanding: bool) {} /// Determine whether the specified URL should be enabled in the Open panel. - fn should_enable_url(&self, url: &str) -> bool { true } + fn should_enable_url(&self, _url: &str) -> bool { true } } diff --git a/appkit/src/menu/item.rs b/appkit/src/menu/item.rs index 824f355..5d6dd17 100644 --- a/appkit/src/menu/item.rs +++ b/appkit/src/menu/item.rs @@ -9,10 +9,15 @@ use objc::{class, msg_send, sel, sel_impl}; use objc::runtime::{Object, Sel}; use objc_id::ShareId; -use crate::events::NSEventModifierFlag; +use crate::events::EventModifierFlag; /// Internal method (shorthand) for generating `NSMenuItem` holders. -fn make_menu_item(title: &str, key: Option<&str>, action: Option, modifier: Option) -> MenuItem { +fn make_menu_item( + title: &str, + key: Option<&str>, + action: Option, + modifiers: Option<&[EventModifierFlag]> +) -> MenuItem { unsafe { let cls = class!(NSMenuItem); let alloc: id = msg_send![cls, alloc]; @@ -29,9 +34,16 @@ fn make_menu_item(title: &str, key: Option<&str>, action: Option, modifier: None => msg_send![alloc, initWithTitle:title action:nil keyEquivalent:key] }); - if let Some(modifier) = modifier { - let _: () = msg_send![&*item, setKeyEquivalentModifierMask:modifier]; - }; + if let Some(modifiers) = modifiers { + let mut key_mask: NSUInteger = 0; + + for modifier in modifiers { + let y: NSUInteger = modifier.into(); + key_mask = key_mask | y; + } + + let _: () = msg_send![&*item, setKeyEquivalentModifierMask:key_mask]; + } MenuItem::Action(item) } @@ -108,7 +120,7 @@ impl MenuItem { "Hide Others", Some("h"), Some(sel!(hide:)), - Some(NSEventModifierFlag::Command | NSEventModifierFlag::Option) + Some(&[EventModifierFlag::Command, EventModifierFlag::Option]) ) } @@ -143,7 +155,7 @@ impl MenuItem { "Enter Full Screen", Some("f"), Some(sel!(toggleFullScreen:)), - Some(NSEventModifierFlag::Command | NSEventModifierFlag::Control) + Some(&[EventModifierFlag::Command, EventModifierFlag::Control]) ) } diff --git a/appkit/src/menu/menu.rs b/appkit/src/menu/menu.rs index eabd1c0..309e181 100644 --- a/appkit/src/menu/menu.rs +++ b/appkit/src/menu/menu.rs @@ -1,6 +1,6 @@ //! Wraps NSMenu and handles instrumenting necessary delegate pieces. -use cocoa::base::{id, nil, YES}; +use cocoa::base::{id, nil}; use cocoa::foundation::NSString; use objc_id::Id; diff --git a/appkit/src/networking/mod.rs b/appkit/src/networking/mod.rs index 85d38af..d7bf120 100644 --- a/appkit/src/networking/mod.rs +++ b/appkit/src/networking/mod.rs @@ -4,7 +4,7 @@ use cocoa::base::id; use objc_id::Id; -use objc::{class, msg_send, sel, sel_impl}; +use objc::{msg_send, sel, sel_impl}; use objc::runtime::Object; use crate::utils::str_from; diff --git a/appkit/src/view/class.rs b/appkit/src/view/class.rs index 914f2ec..ee10ebd 100644 --- a/appkit/src/view/class.rs +++ b/appkit/src/view/class.rs @@ -9,7 +9,7 @@ use std::sync::Once; -use cocoa::base::{id, nil, YES, NO}; +use cocoa::base::{id, nil, YES}; use objc::declare::ClassDecl; use objc::runtime::{Class, Object, Sel, BOOL}; diff --git a/appkit/src/webview/action.rs b/appkit/src/webview/action.rs index 2cd700f..334a5d6 100644 --- a/appkit/src/webview/action.rs +++ b/appkit/src/webview/action.rs @@ -1,10 +1,10 @@ //! Implements wrappers around `WKNavigationAction` and `WKNavigationActionPolicy`. -use cocoa::base::{id, nil, YES, NO}; +use cocoa::base::{id, YES, NO}; use cocoa::foundation::NSInteger; -use objc::runtime::{Class, Object, Sel, BOOL}; -use objc::{class, msg_send, sel, sel_impl}; +use objc::runtime::BOOL; +use objc::{msg_send, sel, sel_impl}; use crate::networking::URLRequest; @@ -73,8 +73,8 @@ impl NavigationResponse { pub fn new(response: id) -> Self { NavigationResponse { can_show_mime_type: unsafe { - let canShow: BOOL = msg_send![response, canShowMIMEType]; - if canShow == YES { true } else { false } + let can_show: BOOL = msg_send![response, canShowMIMEType]; + if can_show == YES { true } else { false } } } } diff --git a/appkit/src/webview/config.rs b/appkit/src/webview/config.rs index 4e9ba03..b1fa81c 100644 --- a/appkit/src/webview/config.rs +++ b/appkit/src/webview/config.rs @@ -1,15 +1,13 @@ //! A wrapper for `WKWebViewConfiguration`. It aims to (mostly) cover //! the important pieces of configuring and updating a WebView configuration. -use cocoa::base::{id, nil, YES, NO}; +use cocoa::base::{id, nil, YES}; use cocoa::foundation::NSString; use objc_id::Id; use objc::runtime::Object; use objc::{class, msg_send, sel, sel_impl}; -use crate::webview::process_pool::register_process_pool; - /// Whether a script should be injected at the start or end of the document load. pub enum InjectAt { Start = 0, diff --git a/appkit/src/webview/controller.rs b/appkit/src/webview/controller.rs index 98dc3b0..3a743d3 100644 --- a/appkit/src/webview/controller.rs +++ b/appkit/src/webview/controller.rs @@ -11,12 +11,12 @@ use cocoa::base::{id, nil, YES, NO}; use cocoa::foundation::{NSRect, NSPoint, NSSize, NSString, NSArray, NSInteger}; use objc::declare::ClassDecl; -use objc::runtime::{Class, Object, Sel, BOOL}; +use objc::runtime::{Class, Object, Sel}; use objc::{class, msg_send, sel, sel_impl}; use crate::ViewController; use crate::view::class::register_view_class; -use crate::webview::action::{NavigationAction, NavigationResponse, OpenPanelParameters}; +use crate::webview::action::{NavigationAction, NavigationResponse}; use crate::webview::{WEBVIEW_VAR, WEBVIEW_CONFIG_VAR, WEBVIEW_CONTROLLER_PTR}; use crate::webview::traits::WebViewController; use crate::utils::str_from; @@ -136,12 +136,9 @@ extern fn decide_policy_for_response(this: &Obje }; let response = NavigationResponse::new(response); - webview.policy_for_navigation_response(response, |policy| { - // This is very sketch and should be heavily checked. :| - unsafe { - let handler = handler as *const Block<(NSInteger,), c_void>; - (*handler).call((policy.into(),)); - } + webview.policy_for_navigation_response(response, |policy| unsafe { + let handler = handler as *const Block<(NSInteger,), c_void>; + (*handler).call((policy.into(),)); }); } @@ -153,28 +150,29 @@ extern fn run_open_panel(this: &Object, _: Sel, &*webview }; - webview.run_open_panel(params.into(), move |urls| { - // This is very sketch and should be heavily checked. :| - unsafe { - let handler = handler as *const Block<(id,), c_void>; + webview.run_open_panel(params.into(), move |urls| unsafe { + let handler = handler as *const Block<(id,), c_void>; - match urls { - Some(u) => { - let nsurls: Vec = u.iter().map(|s| { - let s = NSString::alloc(nil).init_str(&s); - msg_send![class!(NSURL), URLWithString:s] - }).collect(); + match urls { + Some(u) => { + let nsurls: Vec = u.iter().map(|s| { + let s = NSString::alloc(nil).init_str(&s); + msg_send![class!(NSURL), URLWithString:s] + }).collect(); - let array = NSArray::arrayWithObjects(nil, &nsurls); - (*handler).call((array,)); - }, + let array = NSArray::arrayWithObjects(nil, &nsurls); + (*handler).call((array,)); + }, - None => { (*handler).call((nil,)); } - } + None => { (*handler).call((nil,)); } } }); } +/// Called when a download has been initiated in the WebView, and when the navigation policy +/// response is upgraded to BecomeDownload. Only called when explicitly linked since it's a private +/// API. +#[cfg(feature = "enable-webview-downloading")] extern fn handle_download(this: &Object, _: Sel, download: id, suggested_filename: id, handler: usize) { let webview = unsafe { let ptr: usize = *this.get_ivar(WEBVIEW_CONTROLLER_PTR); @@ -182,7 +180,7 @@ extern fn handle_download(this: &Object, _: Sel, &*webview }; - let handler = handler as *const Block<(BOOL, id), c_void>; + let handler = handler as *const Block<(objc::runtime::BOOL, id), c_void>; let filename = str_from(suggested_filename); webview.run_save_panel(filename, move |can_overwrite, path| unsafe { @@ -190,7 +188,6 @@ extern fn handle_download(this: &Object, _: Sel, let _: () = msg_send![download, cancel]; } - println!("Saving to Path: {:?}", path); let path = NSString::alloc(nil).init_str(&path.unwrap()); (*handler).call((match can_overwrite { @@ -236,6 +233,7 @@ pub fn register_controller_class< // WKDownloadDelegate is a private class on macOS that handles downloading (saving) files. // It's absurd that this is still private in 2020. This probably couldn't get into the app // store, so... screw it, fine for now. + #[cfg(feature = "enable-webview-downloading")] decl.add_method(sel!(_download:decideDestinationWithSuggestedFilename:completionHandler:), handle_download:: as extern fn(&Object, _, id, id, usize)); VIEW_CLASS = decl.register(); diff --git a/appkit/src/webview/mod.rs b/appkit/src/webview/mod.rs index 1a9c741..c29310e 100644 --- a/appkit/src/webview/mod.rs +++ b/appkit/src/webview/mod.rs @@ -7,7 +7,7 @@ pub(crate) static WEBVIEW_CONTROLLER_PTR: &str = "rstWebViewControllerPtr"; pub mod action; pub(crate) mod controller; -pub(crate) mod process_pool; +//pub(crate) mod process_pool; pub mod traits; pub use traits::{WebViewController}; diff --git a/appkit/src/webview/traits.rs b/appkit/src/webview/traits.rs index 488cdb6..8c1c7b5 100644 --- a/appkit/src/webview/traits.rs +++ b/appkit/src/webview/traits.rs @@ -1,22 +1,52 @@ +//! WebViewController is a combination of various delegates and helpers for an underlying +//! `WKWebView`. It allows you to do things such as handle opening a file (for uploads or +//! in-browser-processing), handling navigation actions or JS message callbacks, and so on. -use crate::webview::action::{NavigationAction, NavigationPolicy, NavigationResponse, NavigationResponsePolicy, OpenPanelParameters}; +use crate::webview::action::{ + NavigationAction, NavigationPolicy, + NavigationResponse, NavigationResponsePolicy, + OpenPanelParameters +}; +/// You can implement this on structs to handle callbacks from the underlying `WKWebView`. pub trait WebViewController { - fn on_message(&self, name: &str, body: &str) {} + /// Called when a JS message is passed by the browser process. For instance, if you added + /// `notify` as a callback, and in the browser you called + /// `webkit.messageHandlers.notify.postMessage({...})` it would wind up here, with `name` being + /// `notify` and `body` being your arguments. + /// + /// Note that at the moment, you really should handle bridging JSON/stringification yourself. + fn on_message(&self, _name: &str, _body: &str) {} - fn policy_for_navigation_action(&self, action: NavigationAction, handler: F) { + /// Given a callback handler, you can decide what policy should be taken for a given browser + /// action. By default, this is `NavigationPolicy::Allow`. + fn policy_for_navigation_action(&self, _action: NavigationAction, handler: F) { handler(NavigationPolicy::Allow); } - fn policy_for_navigation_response(&self, response: NavigationResponse, handler: F) { + /// Given a callback handler, you can decide what policy should be taken for a given browser + /// response. By default, this is `NavigationResponsePolicy::Allow`. + fn policy_for_navigation_response(&self, _response: NavigationResponse, handler: F) { handler(NavigationResponsePolicy::Allow); } - fn run_open_panel>) + 'static>(&self, parameters: OpenPanelParameters, handler: F) { + /// Given a callback handler and some open panel parameters (e.g, if the user is clicking an + /// upload field that pre-specifies supported options), you should create a `FileSelectPanel` + /// and thread the callbacks accordingly. + fn run_open_panel>) + 'static>(&self, _parameters: OpenPanelParameters, handler: F) { handler(None); } - fn run_save_panel) + 'static>(&self, suggested_filename: &str, handler: F) { + /// Given a callback handler and a suggested filename, you should create a `FileSavePanel` + /// and thread the callbacks accordingly. + /// + /// Note that this specific callback is only + /// automatically fired if you're linking in to the `enable_webview_downloads` feature, which + /// is not guaranteed to be App Store compatible. If you want a version that can go in the App + /// Store, you'll likely need to write some JS in the webview to handle triggering + /// downloading/saving. This is due to Apple not allowing the private methods on `WKWebView` to + /// be open, which... well, complain to them, not me. :) + fn run_save_panel) + 'static>(&self, _suggested_filename: &str, handler: F) { handler(false, None); } } diff --git a/appkit/src/webview/webview.rs b/appkit/src/webview/webview.rs index 530009b..bddfa77 100644 --- a/appkit/src/webview/webview.rs +++ b/appkit/src/webview/webview.rs @@ -12,10 +12,10 @@ use std::rc::Rc; use std::cell::RefCell; use cocoa::base::{id, nil, YES, NO}; -use cocoa::foundation::{NSRect, NSPoint, NSSize, NSString}; +use cocoa::foundation::NSString; use objc_id::ShareId; -use objc::runtime::{Class, Object, Sel, BOOL}; +use objc::runtime::Object; use objc::{class, msg_send, sel, sel_impl}; use crate::ViewController; diff --git a/appkit/src/window/controller.rs b/appkit/src/window/controller.rs index e5aad6e..9d2a2bb 100644 --- a/appkit/src/window/controller.rs +++ b/appkit/src/window/controller.rs @@ -3,11 +3,11 @@ use std::sync::Once; -use cocoa::base::{id, nil, YES, NO}; +use cocoa::base::id; use objc::declare::ClassDecl; use objc::runtime::{Class, Object, Sel}; -use objc::{class, msg_send, sel, sel_impl}; +use objc::{class, sel, sel_impl}; use crate::window::WindowController; @@ -30,7 +30,7 @@ pub(crate) fn register_window_controller_class() static INIT: Once = Once::new(); INIT.call_once(|| unsafe { - let superclass = Class::get("NSWindowController").unwrap(); + let superclass = class!(NSWindowController); let mut decl = ClassDecl::new("RSTWindowController", superclass).unwrap(); decl.add_ivar::(WINDOW_CONTROLLER_PTR); diff --git a/appkit/src/window/window.rs b/appkit/src/window/window.rs index c57deff..fdeef31 100644 --- a/appkit/src/window/window.rs +++ b/appkit/src/window/window.rs @@ -26,9 +26,18 @@ pub struct WindowInner { pub toolbar: Option } -pub mod WindowTitleVisibility { - pub const Visible: usize = 0; - pub const Hidden: usize = 1; +pub enum WindowTitleVisibility { + Visible, + Hidden +} + +impl From for usize { + fn from(visibility: WindowTitleVisibility) -> usize { + match visibility { + WindowTitleVisibility::Visible => 0, + WindowTitleVisibility::Hidden => 1 + } + } } impl WindowInner {