fix(macos): new_window_req not firing on window.open (#1596)

* fix(macos): new_window_req not firing on window.open

currently the new_window_req_handler is fired on navigation events, which is incorrect as it calls the handler on iframe navigations and doesn't call it on window.open calls, not matching behavior on other platforms

this PR changes it to run under [`webView:createWebViewWithConfiguration:forNavigationAction:windowFeatures:`](https://developer.apple.com/documentation/webkit/wkuidelegate/webview(_:createwebviewwith:for:windowfeatures:)?language=objc) instead

I've noticed this when testing tauri-apps/tauri#13876

* fix build

* fix coordinates

* fix features

Author: lucasfernog

.changes/ios-new-window-req-handler.md

@@ -0,0 +1,5 @@
+---
+"wry": patch
+---
+
+Do not fire `new_window_req_handler` on navigation events on macOS and iOS.

.changes/macos-new-window-req-handler.md

@@ -0,0 +1,5 @@
+---
+"wry": patch
+---
+
+Fixes `new_window_req_handler` not fired on macOS when executing `window.open()`.

Cargo.toml

@@ -129,6 +129,7 @@ objc2-web-kit = { version = "0.3.0", default-features = false, features = [
   "WKNavigationResponse",
   "WKUserScript",
   "WKHTTPCookieStore",
+  "WKWindowFeatures",
 ] }
 objc2-core-foundation = { version = "0.3.0", default-features = false, features = [
   "std",
@@ -186,6 +187,8 @@ objc2-app-kit = { version = "0.3.0", default-features = false, features = [
   "NSOpenPanel",
   "NSSavePanel",
   "NSMenu",
+  "NSGraphics",
+  "NSScreen",
 ] }
 
 [target."cfg(target_os = \"android\")".dependencies]

examples/simple.rs

@@ -15,6 +15,10 @@ fn main() -> wry::Result<()> {
 
   let builder = WebViewBuilder::new()
     .with_url("http://tauri.app")
+    .with_new_window_req_handler(|url| {
+      println!("new window req: {url}");
+      true
+    })
     .with_drag_drop_handler(|e| {
       match e {
         wry::DragDropEvent::Enter { paths, position } => {

src/wkwebview/class/wry_navigation_delegate.rs

@@ -32,7 +32,7 @@ use super::wry_download_delegate::WryDownloadDelegate;
 pub struct WryNavigationDelegateIvars {
   pub pending_scripts: Arc<Mutex<Option<Vec<String>>>>,
   pub has_download_handler: bool,
-  pub navigation_policy_function: Box<dyn Fn(String, bool) -> bool>,
+  pub navigation_policy_function: Box<dyn Fn(String) -> bool>,
   pub download_delegate: Option<Retained<WryDownloadDelegate>>,
   pub on_page_load_handler: Option<Box<dyn Fn(PageLoadEvent)>>,
 }
@@ -106,21 +106,14 @@ impl WryNavigationDelegate {
     pending_scripts: Arc<Mutex<Option<Vec<String>>>>,
     has_download_handler: bool,
     navigation_handler: Option<Box<dyn Fn(String) -> bool>>,
-    new_window_req_handler: Option<Box<dyn Fn(String) -> bool>>,
     download_delegate: Option<Retained<WryDownloadDelegate>>,
     on_page_load_handler: Option<Box<dyn Fn(PageLoadEvent, String)>>,
     mtm: MainThreadMarker,
   ) -> Retained<Self> {
-    let navigation_policy_function = Box::new(move |url: String, is_main_frame: bool| -> bool {
-      if is_main_frame {
-        navigation_handler
-          .as_ref()
-          .map_or(true, |navigation_handler| (navigation_handler)(url))
-      } else {
-        new_window_req_handler
-          .as_ref()
-          .map_or(true, |new_window_req_handler| (new_window_req_handler)(url))
-      }
+    let navigation_policy_function = Box::new(move |url: String| -> bool {
+      navigation_handler
+        .as_ref()
+        .map_or(true, |navigation_handler| (navigation_handler)(url))
     });
 
     let on_page_load_handler = if let Some(handler) = on_page_load_handler {

src/wkwebview/class/wry_web_view_ui_delegate.rs

@@ -3,12 +3,14 @@
 // SPDX-License-Identifier: MIT
 
 #[cfg(target_os = "macos")]
-use std::ptr::null_mut;
+use std::{cell::RefCell, ptr::null_mut, sync::Arc};
 
 use block2::Block;
+#[cfg(target_os = "macos")]
+use objc2::DefinedClass;
 use objc2::{define_class, msg_send, rc::Retained, runtime::NSObject, MainThreadOnly};
 #[cfg(target_os = "macos")]
-use objc2_app_kit::{NSModalResponse, NSModalResponseOK, NSOpenPanel};
+use objc2_app_kit::{NSModalResponse, NSModalResponseOK, NSOpenPanel, NSWindowDelegate};
 use objc2_foundation::{MainThreadMarker, NSObjectProtocol};
 #[cfg(target_os = "macos")]
 use objc2_foundation::{NSArray, NSURL};
@@ -21,7 +23,69 @@ use objc2_web_kit::{
 
 use crate::WryWebView;
 
-pub struct WryWebViewUIDelegateIvars {}
+#[cfg(target_os = "macos")]
+struct NewWindow {
+  #[allow(dead_code)]
+  ns_window: Retained<objc2_app_kit::NSWindow>,
+  #[allow(dead_code)]
+  webview: Retained<objc2_web_kit::WKWebView>,
+  #[allow(dead_code)]
+  delegate: Retained<WryNSWindowDelegate>,
+}
+
+// SAFETY: we are not using the new window at all, just dropping it on another thread
+#[cfg(target_os = "macos")]
+unsafe impl Send for NewWindow {}
+
+#[cfg(target_os = "macos")]
+impl Drop for NewWindow {
+  fn drop(&mut self) {
+    unsafe {
+      self.webview.removeFromSuperview();
+    }
+  }
+}
+
+#[cfg(target_os = "macos")]
+struct WryNSWindowDelegateIvars {
+  on_close: Box<dyn Fn()>,
+}
+
+#[cfg(target_os = "macos")]
+define_class!(
+  #[unsafe(super(NSObject))]
+  #[name = "WryNSWindowDelegate"]
+  #[thread_kind = MainThreadOnly]
+  #[ivars = WryNSWindowDelegateIvars]
+  struct WryNSWindowDelegate;
+
+  unsafe impl NSObjectProtocol for WryNSWindowDelegate {}
+
+  unsafe impl NSWindowDelegate for WryNSWindowDelegate {
+    #[unsafe(method(windowWillClose:))]
+    unsafe fn will_close(&self, _notification: &objc2_foundation::NSNotification) {
+      let on_close = &self.ivars().on_close;
+      on_close();
+    }
+  }
+);
+
+#[cfg(target_os = "macos")]
+impl WryNSWindowDelegate {
+  pub fn new(mtm: MainThreadMarker, on_close: Box<dyn Fn()>) -> Retained<Self> {
+    let delegate = mtm
+      .alloc::<WryNSWindowDelegate>()
+      .set_ivars(WryNSWindowDelegateIvars { on_close });
+    unsafe { msg_send![super(delegate), init] }
+  }
+}
+
+pub struct WryWebViewUIDelegateIvars {
+  #[cfg(target_os = "macos")]
+  new_window_req_handler: Option<Box<dyn Fn(String) -> bool>>,
+  #[cfg(target_os = "macos")]
+  new_windows: Arc<RefCell<Vec<NewWindow>>>,
+}
 
 define_class!(
   #[unsafe(super(NSObject))]
@@ -73,14 +137,123 @@ define_class!(
       //https://developer.apple.com/documentation/webkit/wkpermissiondecision?language=objc
       (*decision_handler).call((WKPermissionDecision::Grant,));
     }
+
+    #[cfg(target_os = "macos")]
+    #[unsafe(method_id(webView:createWebViewWithConfiguration:forNavigationAction:windowFeatures:))]
+    unsafe fn create_web_view_for_navigation_action(
+      &self,
+      webview: &WryWebView,
+      configuration: &objc2_web_kit::WKWebViewConfiguration,
+      action: &objc2_web_kit::WKNavigationAction,
+      window_features: &objc2_web_kit::WKWindowFeatures,
+    ) -> Option<Retained<objc2_web_kit::WKWebView>> {
+      if let Some(new_window_req_handler) = &self.ivars().new_window_req_handler {
+        let request = action.request();
+        let url = request.URL().unwrap().absoluteString().unwrap();
+        let create = new_window_req_handler(url.to_string());
+        if create {
+          let mtm = MainThreadMarker::new().unwrap();
+
+          let current_window = webview.window().unwrap();
+          let screen = current_window.screen().unwrap();
+          let screen_frame = screen.frame();
+          let defaults = current_window.frame();
+          let size = objc2_foundation::NSSize::new(
+            window_features
+              .width()
+              .map_or(defaults.size.width, |width| width.doubleValue()),
+            window_features
+              .height()
+              .map_or(defaults.size.height, |height| height.doubleValue()),
+          );
+          let position = objc2_foundation::NSPoint::new(
+            window_features
+              .x()
+              .map_or(defaults.origin.x, |x| x.doubleValue()),
+            window_features.y().map_or(defaults.origin.y, |y| {
+              screen_frame.size.height - y.doubleValue() - size.height
+            }),
+          );
+          let rect = objc2_foundation::NSRect::new(position, size);
+
+          let mut flags = objc2_app_kit::NSWindowStyleMask::Titled
+            | objc2_app_kit::NSWindowStyleMask::Closable
+            | objc2_app_kit::NSWindowStyleMask::Miniaturizable;
+          let resizable = window_features
+            .allowsResizing()
+            .map_or(true, |resizable| resizable.boolValue());
+          if resizable {
+            flags |= objc2_app_kit::NSWindowStyleMask::Resizable;
+          }
+
+          let window = objc2_app_kit::NSWindow::initWithContentRect_styleMask_backing_defer(
+            mtm.alloc::<objc2_app_kit::NSWindow>(),
+            rect,
+            flags,
+            objc2_app_kit::NSBackingStoreType::Buffered,
+            false,
+          );
+
+          // SAFETY: Disable auto-release when closing windows.
+          // This is required when creating `NSWindow` outside a window
+          // controller.
+          window.setReleasedWhenClosed(false);
+
+          let webview = objc2_web_kit::WKWebView::initWithFrame_configuration(
+            mtm.alloc::<objc2_web_kit::WKWebView>(),
+            window.frame(),
+            configuration,
+          );
+
+          let new_windows = self.ivars().new_windows.clone();
+          let window_id = Retained::as_ptr(&window) as usize;
+          let delegate = WryNSWindowDelegate::new(
+            mtm,
+            Box::new(move || {
+              let new_windows = new_windows.clone();
+              new_windows
+                .borrow_mut()
+                .retain(|window| Retained::as_ptr(&window.ns_window) as usize != window_id);
+            }),
+          );
+          window.setDelegate(Some(objc2::runtime::ProtocolObject::from_ref(&*delegate)));
+
+          window.setContentView(Some(&webview));
+          window.makeKeyAndOrderFront(None);
+
+          self.ivars().new_windows.borrow_mut().push(NewWindow {
+            ns_window: window,
+            webview: webview.clone(),
+            delegate,
+          });
+
+          Some(webview)
+        } else {
+          None
+        }
+      } else {
+        None
+      }
+    }
   }
 );
 
 impl WryWebViewUIDelegate {
-  pub fn new(mtm: MainThreadMarker) -> Retained<Self> {
+  pub fn new(
+    mtm: MainThreadMarker,
+    new_window_req_handler: Option<Box<dyn Fn(String) -> bool>>,
+  ) -> Retained<Self> {
+    #[cfg(target_os = "ios")]
+    let _new_window_req_handler = new_window_req_handler;
+
     let delegate = mtm
       .alloc::<WryWebViewUIDelegate>()
-      .set_ivars(WryWebViewUIDelegateIvars {});
+      .set_ivars(WryWebViewUIDelegateIvars {
+        #[cfg(target_os = "macos")]
+        new_window_req_handler,
+        #[cfg(target_os = "macos")]
+        new_windows: Arc::new(RefCell::new(vec![])),
+      });
     unsafe { msg_send![super(delegate), init] }
   }
 }

src/wkwebview/mod.rs

@@ -515,7 +515,6 @@ impl InnerWebView {
         pending_scripts.clone(),
         has_download_handler,
         attributes.navigation_handler,
-        attributes.new_window_req_handler,
         download_delegate.clone(),
         attributes.on_page_load_handler,
         mtm,
@@ -524,7 +523,8 @@ impl InnerWebView {
       let proto_navigation_policy_delegate = ProtocolObject::from_ref(&*navigation_policy_delegate);
       webview.setNavigationDelegate(Some(proto_navigation_policy_delegate));
 
-      let ui_delegate: Retained<WryWebViewUIDelegate> = WryWebViewUIDelegate::new(mtm);
+      let ui_delegate: Retained<WryWebViewUIDelegate> =
+        WryWebViewUIDelegate::new(mtm, attributes.new_window_req_handler);
       let proto_ui_delegate = ProtocolObject::from_ref(&*ui_delegate);
       webview.setUIDelegate(Some(proto_ui_delegate));
 

src/wkwebview/navigation.rs

@@ -63,8 +63,6 @@ pub(crate) fn navigation_policy(
     };
     let request = action.request();
     let url = request.URL().unwrap().absoluteString().unwrap();
-    let target_frame = action.targetFrame();
-    let is_main_frame = target_frame.is_some_and(|frame| frame.isMainFrame());
 
     if should_download {
       let has_download_handler = this.ivars().has_download_handler;
@@ -75,7 +73,7 @@ pub(crate) fn navigation_policy(
       }
     } else {
       let function = &this.ivars().navigation_policy_function;
-      match function(url.to_string(), is_main_frame) {
+      match function(url.to_string()) {
         true => (*handler).call((WKNavigationActionPolicy::Allow,)),
         false => (*handler).call((WKNavigationActionPolicy::Cancel,)),
       };