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

refactor(macos): migrate to objc2 #1316

Draft
wants to merge 39 commits into
base: dev
Choose a base branch
from
Draft

Conversation

pewsheen
Copy link
Contributor

@pewsheen pewsheen commented Jul 10, 2024

Issue: #1239

Roadmap

  • Remove objc dependency
  • Merge dev branch
  • Declare Custom Class
    • WryWebView
    • {}URLSchemeHandler (We need to create it dynamically)
    • WebViewDelegate -> WryWebViewDelegate
    • DocumentTitleChangedDelegate
    • WryNavigationDelegate
    • WryDownloadDelegate
    • WebViewUIDelegate -> WryWebViewUIDelegate
    • WryWebViewParent
  • Test Functionality
  • Update Tauri to use wry with objc2 pewsheen/refactor/wry-objc2

Functionality Test:

  • Window (single webview)
  • Window (multiple webviews)
  • Window iOS
  • PerformKeyEquivalent (menu shortcut)
  • Picture in picture
  • Fullscreen (feature = fullscreen)
  • Document title changed handler
  • Navigation Policy (download handler?)
  • [mac] Reparent
  • UIDelegate
    • File upload
    • Media capture permission
  • [ios] setAllowsInlineMediaPlayback (not merged yet, in dev)
  • [mac] Synthetic mouse event
  • [mac] Tab focuses links
  • with_*
    • [mac] with_back_forward_navigation_gestures
    • with_transparent
    • with_visible
    • with_autoplay (false is not working, dev as well)
    • with_initialization_script
    • with_custom_protocol
    • with_asynchronous_custom_protocol
    • with_ipc_handler
    • [mac] with_drag_drop_handler
    • with_url_and_headers
    • with_url
    • with_headers
    • with_html
    • with_user_agent
    • with_devtools
    • with_navigation_handler
    • with_download_started_handler
    • with_download_completed_handler
    • with_new_window_req_handler
    • [mac] with_accept_first_mouse
    • with_document_title_changed_handler
    • with_incognito
    • with_on_page_load_handler
    • with_proxy_config (feature = mac-proxy)
    • with_bounds (child webview)
  • pub fn
    • [mac] print
    • url
    • load_url
    • load_url_with_headers
    • eval
    • clear_all_browsing_data
    • [mac] open_devtools
    • [mac] close_devtools
    • [mac] is_devtools_open
    • zoom
    • bounds
    • [mac] set_bounds (child webview)
    • set_visible
    • focus
    • url_from_webview
    • platform_webview_version

Issues

if catch_unwind(|| {
  config_unwind_safe.setURLSchemeHandler_forURLScheme(
    Some(&*(handler_unwind_safe.cast::<ProtocolObject<dyn WKURLSchemeHandler>>())),
    &NSString::from_str(&name),
  );
})

Copy link
Contributor

github-actions bot commented Jul 10, 2024

Package Changes Through d0f6990

No changes.

Add a change file through the GitHub UI by following this link.


Read about change files or the docs at github.com/jbolda/covector

@pewsheen pewsheen force-pushed the refactor/migrate-to-objc2 branch 3 times, most recently from 17287ab to fdd1534 Compare July 10, 2024 10:41
Copy link

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gave it a cursory look, great overall, really nice to see objc2 used effectively, gives me motivation to fix the places where it isn't working!

Cargo.toml Outdated Show resolved Hide resolved
"NSBundle",
"NSProcessInfo",
] }
objc2-app-kit = { version = "0.2.0", features = [
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should only be enabled under [target."cfg(target_os = \"macos\")".dependencies]. Reversed for objc2-ui-kit.

objc2-web-kit = { version = "0.2.0", features = [
"objc2-app-kit",
"block2",
"WKWebView",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

objc2-web-kit not support WKWebView on iOS (madsmtm/objc2#637)

I think the easiest approach for now might be for you to copy objc2-web-kit's definition from the source into a file in this repo, remove the #[cfg(feature = "objc2-app-kit")], change NSWindow to UIWindow, and import it from that file on iOS instead of from objc2-web-kit.

src/wkwebview/drag_drop.rs Outdated Show resolved Hide resolved
src/wkwebview/class/wry_web_view.rs Outdated Show resolved Hide resolved
src/wkwebview/class/wry_web_view.rs Outdated Show resolved Hide resolved
Comment on lines 69 to 70
download_started_handler: Option<Box<dyn FnMut(String, &mut PathBuf) -> bool>>,
download_completed_handler: Option<Rc<dyn Fn(String, Option<PathBuf>, bool)>>,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you change these to take dyn Fn(...) + 'static instead, then you don't need to into_raw and from_raw it, and it also becomes sound. (At least I think that's how the lifetimes work here? I can never remember...)

Also, Box<dyn FnMut(...)> really isn't possible in Objective-C classes, you'll have to wrap it in a RefCell.

Suggested change
download_started_handler: Option<Box<dyn FnMut(String, &mut PathBuf) -> bool>>,
download_completed_handler: Option<Rc<dyn Fn(String, Option<PathBuf>, bool)>>,
download_started_handler: Option<RefCell<Box<dyn FnMut(String, &mut PathBuf) -> bool + 'static>>>,
download_completed_handler: Option<Rc<dyn Fn(String, Option<PathBuf>, bool) + 'static>>,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, Box<dyn FnMut(...)> really isn't possible in Objective-C classes, you'll have to wrap it in a RefCell.

Could you explain more about this? I am curious how this work

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Objective-C classes require interior mutability (&self), so while you can store Box<dyn FnMut(...)> in them, it'd be useless, as it requires a mutable reference (&mut self) to call the closure.

It'd be kinda like using Rc<Box<dyn FnMut(...)>> - try it out, it won't work unless you introduce RefCell or similar forms of interior mutability.

Just to clear up any confusion, Box<dyn Fn(...)> is fine, it's the FnMut that's problematic.

src/wkwebview/class/url_scheme_handler.rs Outdated Show resolved Hide resolved
src/wkwebview/mod.rs Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@pewsheen
Copy link
Contributor Author

pewsheen commented Sep 8, 2024

Note: iOS can now be built and run on the iOS simulator from Tauri (it requires some modification in Tauri to use objc2). It's still WIP for testing some functionalities and organizing the code.

https://github.com/pewsheen/tauri/tree/refactor/wry-objc2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants