bug#60703: Patches to xwidget code

bug#60703: Patches to xwidget code

[Andrew De Angelis]

[-- Attachment #1.1: Type: text/plain, Size: 1561 bytes --]

Hello everyone and thanks for all your work!

I have some fixes to the xwidget code that I'd like to contribute.

These should achieve three things
1) fix `xwidget-webkit-current-url' in xwidget.el so that it actually
displays the current URL
2) fix the Objective-C code in nsxwidget.m to eliminate memory leaks
3) implement the function `xwidget-webkit-estimated-load-progress' in
Objective-C, making this functionality available for macOS as well

Regarding 2), I have tested my changes with the Instruments app within the
XCode dev tools. I was able to test them on two different machines: a
MacBook Air M2 running macOS Ventura 13, and an Intel processor running
macOS Big Sur 11.6.7.
When testing with Instruments, it appears that Emacs isn't leaking memory
anymore. I am still seeing some leaks, but according to Instruments, the
responsible libraries are system libraries, and not Emacs itself.
If possible, I would appreciate it if other people can test/profile these
changes as well, and share their feedback.

Assuming the changes are correct and accepted, would it be possible to
include them in the upcoming Emacs 29.1 release?
Technically, none of these are new features; it's just fixes to existing
features.

I intend to keep working on xwidget support for macOS (fixing remaining
issues and implementing missing functionalities), but I'm not sure when
I'll be able to get to it; for now I'd like to start by contributing these
first major fixes.

Thanks in advance for your feedback; let me know if you need anything else
from me.

Best,
Andrew

[-- Attachment #1.2: Type: text/html, Size: 1876 bytes --]

[-- Attachment #2: xwidget-patches.patch --]
[-- Type: application/octet-stream, Size: 10051 bytes --]

diff --git a/lisp/xwidget.el b/lisp/xwidget.el
index abbda29081..8095fa9db5 100644
--- a/lisp/xwidget.el
+++ b/lisp/xwidget.el
@@ -924,8 +924,9 @@ xwidget-webkit-reload
 (defun xwidget-webkit-current-url ()
   "Display the current xwidget webkit URL and place it on the `kill-ring'."
   (interactive nil xwidget-webkit-mode)
-  (let ((url (xwidget-webkit-uri (xwidget-webkit-current-session))))
-    (message "URL: %s" (kill-new (or url "")))))
+  (let ((url (or (xwidget-webkit-uri (xwidget-webkit-current-session)) "")))
+    (kill-new url)
+    (message "URL: %s" url)))
 
 (defun xwidget-webkit-browse-history ()
   "Display a buffer containing the history of page loads."
diff --git a/src/nsxwidget.h b/src/nsxwidget.h
index 8d55fac532..2b5596f905 100644
--- a/src/nsxwidget.h
+++ b/src/nsxwidget.h
@@ -36,6 +36,8 @@ #define NSXWIDGET_H_INCLUDED
 Lisp_Object nsxwidget_webkit_title (struct xwidget *xw);
 void nsxwidget_webkit_goto_uri (struct xwidget *xw, const char *uri);
 void nsxwidget_webkit_goto_history (struct xwidget *xw, int rel_pos);
+double nsxwidget_webkit_estimated_load_progress(struct xwidget *xw);
+void nsxwidget_webkit_stop_loading (struct xwidget *xw);
 void nsxwidget_webkit_zoom (struct xwidget *xw, double zoom_change);
 void nsxwidget_webkit_execute_script (struct xwidget *xw, const char *script,
                                       Lisp_Object fun);
diff --git a/src/nsxwidget.m b/src/nsxwidget.m
index e1fbd749b6..aaf867bcdd 100644
--- a/src/nsxwidget.m
+++ b/src/nsxwidget.m
@@ -57,12 +57,18 @@ @interface XwWebView : WKWebView
 @end
 @implementation XwWebView : WKWebView
 
+- (void)dealloc
+{
+  [super dealloc];
+}
+
 - (id)initWithFrame:(CGRect)frame
       configuration:(WKWebViewConfiguration *)configuration
             xwidget:(struct xwidget *)xw
 {
   /* Script controller to add script message handler and user script.  */
-  WKUserContentController *scriptor = [[WKUserContentController alloc] init];
+  WKUserContentController *scriptor = [[[WKUserContentController alloc] init]
+                                        autorelease];
   configuration.userContentController = scriptor;
 
   /* Enable inspect element context menu item for debugging.  */
@@ -81,7 +87,8 @@ - (id)initWithFrame:(CGRect)frame
   if (self)
     {
       self.xw = xw;
-      self.urlScriptBlocked = [[NSMutableDictionary alloc] init];
+      self.urlScriptBlocked = [[[NSMutableDictionary alloc] init]
+                                autorelease];
       self.navigationDelegate = self;
       self.UIDelegate = self;
       self.customUserAgent =
@@ -89,11 +96,13 @@ - (id)initWithFrame:(CGRect)frame
         @" AppleWebKit/603.3.8 (KHTML, like Gecko)"
         @" Version/11.0.1 Safari/603.3.8";
       [scriptor addScriptMessageHandler:self name:@"keyDown"];
-      [scriptor addUserScript:[[WKUserScript alloc]
-                                initWithSource:xwScript
-                                 injectionTime:
-                                  WKUserScriptInjectionTimeAtDocumentStart
-                                forMainFrameOnly:NO]];
+      WKUserScript *userScript = [[[WKUserScript alloc]
+                                    initWithSource:xwScript
+                                     injectionTime:
+                                      WKUserScriptInjectionTimeAtDocumentStart
+                                    forMainFrameOnly:NO]
+                                   autorelease];
+      [scriptor addUserScript:userScript];
     }
   return self;
 }
@@ -102,7 +111,27 @@ - (void)webView:(WKWebView *)webView
 didFinishNavigation:(WKNavigation *)navigation
 {
   if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
-    store_xwidget_event_string (self.xw, "load-changed", "");
+    store_xwidget_event_string (self.xw, "load-changed", "load-finished");
+}
+
+- (void)webView:(WKWebView *)webView didStartProvisionalNavigation:(WKNavigation *)navigation
+{
+  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
+    store_xwidget_event_string (self.xw, "load-changed", "load-started");
+}
+
+- (void)webView:(WKWebView *)webView
+didReceiveServerRedirectForProvisionalNavigation:(WKNavigation *)navigation
+{
+  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
+    store_xwidget_event_string (self.xw, "load-changed", "load-redirected");
+}
+
+// Start loading WKWebView
+- (void)webView:(WKWebView *)webView didCommitNavigation:(WKNavigation *)navigation
+{
+  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt)) // what exactly is this test for
+    store_xwidget_event_string (self.xw, "load-changed", "load-committed");
 }
 
 - (void)webView:(WKWebView *)webView
@@ -343,6 +372,20 @@ - (void)userContentController:(WKUserContentController *)userContentController
   }
 }
 
+double
+nsxwidget_webkit_estimated_load_progress(struct xwidget *xw)
+{
+  XwWebView *xwWebView = (XwWebView *) xw->xwWidget;
+  return xwWebView.estimatedProgress;
+}
+
+void
+nsxwidget_webkit_stop_loading (struct xwidget *xw)
+{
+  XwWebView *xwWebView = (XwWebView *) xw->xwWidget;
+  [xwWebView stopLoading];
+}
+
 void
 nsxwidget_webkit_zoom (struct xwidget *xw, double zoom_change)
 {
@@ -452,7 +495,8 @@ - (BOOL)isFlipped { return YES; }
   NSRect rect = NSMakeRect (0, 0, xw->width, xw->height);
   xw->xwWidget = [[XwWebView alloc]
                    initWithFrame:rect
-                   configuration:[[WKWebViewConfiguration alloc] init]
+                   configuration:[[[WKWebViewConfiguration alloc] init]
+                                   autorelease]
                          xwidget:xw];
   xw->xwWindow = [[XwWindow alloc]
                    initWithFrame:rect];
@@ -470,16 +514,18 @@ - (BOOL)isFlipped { return YES; }
         ((XwWebView *) xw->xwWidget).configuration.userContentController;
       [scriptor removeAllUserScripts];
       [scriptor removeScriptMessageHandlerForName:@"keyDown"];
-      [scriptor release];
+
       if (xw->xv)
         xw->xv->model = Qnil; /* Make sure related view stale.  */
 
       /* This stops playing audio when a xwidget-webkit buffer is
-         killed.  I could not find other solution.  */
+         killed.  I could not find other solution.
+         TODO: improve this */
       nsxwidget_webkit_goto_uri (xw, "about:blank");
 
       [((XwWebView *) xw->xwWidget).urlScriptBlocked release];
       [xw->xwWidget removeFromSuperviewWithoutNeedingDisplay];
+
       [xw->xwWidget release];
       [xw->xwWindow removeFromSuperviewWithoutNeedingDisplay];
       [xw->xwWindow release];
diff --git a/src/xwidget.c b/src/xwidget.c
index efe2705562..b9550da460 100644
--- a/src/xwidget.c
+++ b/src/xwidget.c
@@ -54,6 +54,7 @@ Copyright (C) 2011-2023 Free Software Foundation, Inc.
 
 #include <math.h>
 
+
 static Lisp_Object id_to_xwidget_map;
 static Lisp_Object internal_xwidget_view_list;
 static Lisp_Object internal_xwidget_list;
@@ -490,10 +491,10 @@ DEFUN ("xwidget-perform-lispy-event",
 {
   struct xwidget *xw;
   struct frame *f = NULL;
-  int character = -1, keycode = -1;
-  int modifiers = 0;
 
 #ifdef USE_GTK
+  int character = -1, keycode = -1;
+  int modifiers = 0;
   GdkEvent *xg_event;
   GtkContainerClass *klass;
   GtkWidget *widget;
@@ -3063,6 +3064,36 @@ DEFUN ("xwidget-webkit-title",
 #endif
 }
 
+DEFUN ("xwidget-webkit-estimated-load-progress",
+       Fxwidget_webkit_estimated_load_progress, Sxwidget_webkit_estimated_load_progress,
+       1, 1, 0, doc: /* Get the estimated load progress of XWIDGET, a WebKit widget.
+Return a value ranging from 0.0 to 1.0, based on how close XWIDGET
+is to completely loading its page.  */)
+  (Lisp_Object xwidget)
+{
+  struct xwidget *xw;
+#ifdef USE_GTK
+  WebKitWebView *webview;
+#endif
+  double value;
+
+  CHECK_LIVE_XWIDGET (xwidget);
+  xw = XXWIDGET (xwidget);
+  CHECK_WEBKIT_WIDGET (xw);
+
+  block_input ();
+#ifdef USE_GTK
+  webview = WEBKIT_WEB_VIEW (xw->widget_osr);
+  value = webkit_web_view_get_estimated_load_progress (webview);
+#elif defined NS_IMPL_COCOA
+  value = nsxwidget_webkit_estimated_load_progress (xw);
+#endif
+
+  unblock_input ();
+
+  return make_float (value);
+}
+
 DEFUN ("xwidget-webkit-goto-uri",
        Fxwidget_webkit_goto_uri, Sxwidget_webkit_goto_uri,
        2, 2, 0,
@@ -3810,28 +3841,6 @@ DEFUN ("xwidget-webkit-back-forward-list", Fxwidget_webkit_back_forward_list,
   return list3 (back, here, forward);
 }
 
-DEFUN ("xwidget-webkit-estimated-load-progress",
-       Fxwidget_webkit_estimated_load_progress, Sxwidget_webkit_estimated_load_progress,
-       1, 1, 0, doc: /* Get the estimated load progress of XWIDGET, a WebKit widget.
-Return a value ranging from 0.0 to 1.0, based on how close XWIDGET
-is to completely loading its page.  */)
-  (Lisp_Object xwidget)
-{
-  struct xwidget *xw;
-  WebKitWebView *webview;
-  double value;
-
-  CHECK_LIVE_XWIDGET (xwidget);
-  xw = XXWIDGET (xwidget);
-  CHECK_WEBKIT_WIDGET (xw);
-
-  block_input ();
-  webview = WEBKIT_WEB_VIEW (xw->widget_osr);
-  value = webkit_web_view_get_estimated_load_progress (webview);
-  unblock_input ();
-
-  return make_float (value);
-}
 #endif
 
 DEFUN ("xwidget-webkit-set-cookie-storage-file",
@@ -3874,19 +3883,23 @@ DEFUN ("xwidget-webkit-stop-loading", Fxwidget_webkit_stop_loading,
 XWIDGET as part of loading a page.  */)
   (Lisp_Object xwidget)
 {
-#ifdef USE_GTK
   struct xwidget *xw;
+#ifdef USE_GTK
   WebKitWebView *webview;
+#endif
 
   CHECK_LIVE_XWIDGET (xwidget);
   xw = XXWIDGET (xwidget);
   CHECK_WEBKIT_WIDGET (xw);
 
   block_input ();
+#ifdef USE_GTK
   webview = WEBKIT_WEB_VIEW (xw->widget_osr);
   webkit_web_view_stop_loading (webview);
-  unblock_input ();
+#elif defined NS_IMPL_COCOA
+  nsxwidget_webkit_stop_loading (xw);
 #endif
+  unblock_input ();
 
   return Qnil;
 }
@@ -3936,8 +3949,9 @@ syms_of_xwidget (void)
 #ifdef USE_GTK
   defsubr (&Sxwidget_webkit_load_html);
   defsubr (&Sxwidget_webkit_back_forward_list);
-  defsubr (&Sxwidget_webkit_estimated_load_progress);
 #endif
+
+  defsubr (&Sxwidget_webkit_estimated_load_progress);
   defsubr (&Skill_xwidget);
 
   DEFSYM (QCxwidget, ":xwidget");

Re: bug#60703: Patches to xwidget code

[Po Lu]

Andrew De Angelis <bobodeangelis@gmail.com> writes:

> Hello everyone and thanks for all your work!
>
> I have some fixes to the xwidget code that I'd like to contribute.
>
> These should achieve three things 1) fix `xwidget-webkit-current-url'
> in xwidget.el so that it actually displays the current URL 2) fix the
> Objective-C code in nsxwidget.m to eliminate memory leaks 3) implement
> the function `xwidget-webkit-estimated-load-progress' in Objective-C,
> making this functionality available for macOS as well
>
> Regarding 2), I have tested my changes with the Instruments app within
> the XCode dev tools. I was able to test them on two different
> machines: a MacBook Air M2 running macOS Ventura 13, and an Intel
> processor running macOS Big Sur 11.6.7.  When testing with
> Instruments, it appears that Emacs isn't leaking memory anymore. I am
> still seeing some leaks, but according to Instruments, the responsible
> libraries are system libraries, and not Emacs itself.  If possible, I
> would appreciate it if other people can test/profile these changes as
> well, and share their feedback.
>
> Assuming the changes are correct and accepted, would it be possible to
> include them in the upcoming Emacs 29.1 release?  Technically, none of
> these are new features; it's just fixes to existing features.
>
> I intend to keep working on xwidget support for macOS (fixing
> remaining issues and implementing missing functionalities), but I'm
> not sure when I'll be able to get to it; for now I'd like to start by
> contributing these first major fixes.
>
> Thanks in advance for your feedback; let me know if you need anything
> else from me.

Andrew, thanks for working on Emacs.

>
> diff --git a/lisp/xwidget.el b/lisp/xwidget.el
> index abbda29081..8095fa9db5 100644
> --- a/lisp/xwidget.el
> +++ b/lisp/xwidget.el
> @@ -924,8 +924,9 @@ xwidget-webkit-reload
>  (defun xwidget-webkit-current-url ()
>    "Display the current xwidget webkit URL and place it on the `kill-ring'."
>    (interactive nil xwidget-webkit-mode)
> -  (let ((url (xwidget-webkit-uri (xwidget-webkit-current-session))))
> -    (message "URL: %s" (kill-new (or url "")))))
> +  (let ((url (or (xwidget-webkit-uri (xwidget-webkit-current-session)) "")))
> +    (kill-new url)
> +    (message "URL: %s" url)))
>  
>  (defun xwidget-webkit-browse-history ()
>    "Display a buffer containing the history of page loads."

This change is fine for Emacs 29.1.  Eli, WDYT?

>  @implementation XwWebView : WKWebView
>  
> +- (void)dealloc
> +{
> +  [super dealloc];
> +}

Thanks.  Please put a space between the part of the Objective-C method
that looks like a cast (I don't know what it's called, sorry) and the
name of the method.

But, is this really necessary?  Without this being defined, the runtime
will just call the super method, correct?

>  - (id)initWithFrame:(CGRect)frame
>        configuration:(WKWebViewConfiguration *)configuration
>              xwidget:(struct xwidget *)xw
>  {
>    /* Script controller to add script message handler and user script.  */
> -  WKUserContentController *scriptor = [[WKUserContentController alloc] init];
> +  WKUserContentController *scriptor = [[[WKUserContentController alloc] init]
> +                                        autorelease];
>    configuration.userContentController = scriptor;
>  
>    /* Enable inspect element context menu item for debugging.  */
> @@ -81,7 +87,8 @@ - (id)initWithFrame:(CGRect)frame
>    if (self)
>      {
>        self.xw = xw;
> -      self.urlScriptBlocked = [[NSMutableDictionary alloc] init];
> +      self.urlScriptBlocked = [[[NSMutableDictionary alloc] init]
> +                                autorelease];
>        self.navigationDelegate = self;
>        self.UIDelegate = self;
>        self.customUserAgent =
> @@ -89,11 +96,13 @@ - (id)initWithFrame:(CGRect)frame
>          @" AppleWebKit/603.3.8 (KHTML, like Gecko)"
>          @" Version/11.0.1 Safari/603.3.8";
>        [scriptor addScriptMessageHandler:self name:@"keyDown"];
> -      [scriptor addUserScript:[[WKUserScript alloc]
> -                                initWithSource:xwScript
> -                                 injectionTime:
> -                                  WKUserScriptInjectionTimeAtDocumentStart
> -                                forMainFrameOnly:NO]];
> +      WKUserScript *userScript = [[[WKUserScript alloc]
> +                                    initWithSource:xwScript
> +                                     injectionTime:
> +                                      WKUserScriptInjectionTimeAtDocumentStart
> +                                    forMainFrameOnly:NO]
> +                                   autorelease];
> +      [scriptor addUserScript:userScript];
>      }
>    return self;
>  }
> @@ -102,7 +111,27 @@ - (void)webView:(WKWebView *)webView
>  didFinishNavigation:(WKNavigation *)navigation
>  {
>    if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
> -    store_xwidget_event_string (self.xw, "load-changed", "");
> +    store_xwidget_event_string (self.xw, "load-changed", "load-finished");
> +}
> +
> +- (void)webView:(WKWebView *)webView didStartProvisionalNavigation:(WKNavigation *)navigation
> +{
> +  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
> +    store_xwidget_event_string (self.xw, "load-changed", "load-started");
> +}
> +
> +- (void)webView:(WKWebView *)webView
> +didReceiveServerRedirectForProvisionalNavigation:(WKNavigation *)navigation
> +{
> +  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
> +    store_xwidget_event_string (self.xw, "load-changed", "load-redirected");
> +}
> +
> +// Start loading WKWebView
> +- (void)webView:(WKWebView *)webView didCommitNavigation:(WKNavigation *)navigation
> +{
> +  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt)) // what exactly is this test for
> +    store_xwidget_event_string (self.xw, "load-changed", "load-committed");
>  }

These are also fine for Emacs 29.  However, please replace the C++-style
comments with C style ones, add spaces after the cast-like parts in the
method definition, and fill everything to column 80.  I believe the test
is that the xwidget's buffer has not been killed, and could easily be
written:

  !NILP (Fbuffer_live_p (self.xw->buffer))

as well.

>  - (void)webView:(WKWebView *)webView
> @@ -343,6 +372,20 @@ - (void)userContentController:(WKUserContentController *)userContentController
>    }
>  }
>  
> +double
> +nsxwidget_webkit_estimated_load_progress(struct xwidget *xw)
> +{
> +  XwWebView *xwWebView = (XwWebView *) xw->xwWidget;
> +  return xwWebView.estimatedProgress;
> +}

Please place a space between "nsxwidget_webkit_estimated_load_progress"
and the parameter list.

> +void
> +nsxwidget_webkit_stop_loading (struct xwidget *xw)
> +{
> +  XwWebView *xwWebView = (XwWebView *) xw->xwWidget;
> +  [xwWebView stopLoading];
> +}
> +
>  void
>  nsxwidget_webkit_zoom (struct xwidget *xw, double zoom_change)
>  {
> @@ -452,7 +495,8 @@ - (BOOL)isFlipped { return YES; }
>    NSRect rect = NSMakeRect (0, 0, xw->width, xw->height);
>    xw->xwWidget = [[XwWebView alloc]
>                     initWithFrame:rect
> -                   configuration:[[WKWebViewConfiguration alloc] init]
> +                   configuration:[[[WKWebViewConfiguration alloc] init]
> +                                   autorelease]
>                           xwidget:xw];
>    xw->xwWindow = [[XwWindow alloc]
>                     initWithFrame:rect];
> @@ -470,16 +514,18 @@ - (BOOL)isFlipped { return YES; }
>          ((XwWebView *) xw->xwWidget).configuration.userContentController;
>        [scriptor removeAllUserScripts];
>        [scriptor removeScriptMessageHandlerForName:@"keyDown"];
> -      [scriptor release];
> +
>        if (xw->xv)
>          xw->xv->model = Qnil; /* Make sure related view stale.  */
>  
>        /* This stops playing audio when a xwidget-webkit buffer is
> -         killed.  I could not find other solution.  */
> +         killed.  I could not find other solution.
> +         TODO: improve this */
>        nsxwidget_webkit_goto_uri (xw, "about:blank");
>  
>        [((XwWebView *) xw->xwWidget).urlScriptBlocked release];
>        [xw->xwWidget removeFromSuperviewWithoutNeedingDisplay];
> +
>        [xw->xwWidget release];
>        [xw->xwWindow removeFromSuperviewWithoutNeedingDisplay];
>        [xw->xwWindow release];

These changes are also fine for Emacs 29, but the TODO seems excessive.
Eli, any comments here?

> diff --git a/src/xwidget.c b/src/xwidget.c
> index efe2705562..b9550da460 100644
> --- a/src/xwidget.c
> +++ b/src/xwidget.c
> @@ -54,6 +54,7 @@ Copyright (C) 2011-2023 Free Software Foundation, Inc.
>  
>  #include <math.h>
>  
> +

Please undo this extraneous whitespace change.

>  
> +DEFUN ("xwidget-webkit-estimated-load-progress",
> +       Fxwidget_webkit_estimated_load_progress, Sxwidget_webkit_estimated_load_progress,
> +       1, 1, 0, doc: /* Get the estimated load progress of XWIDGET, a WebKit widget.
> +Return a value ranging from 0.0 to 1.0, based on how close XWIDGET
> +is to completely loading its page.  */)
> +  (Lisp_Object xwidget)
> +{
> +  struct xwidget *xw;
> +#ifdef USE_GTK
> +  WebKitWebView *webview;
> +#endif
> +  double value;
> +
> +  CHECK_LIVE_XWIDGET (xwidget);
> +  xw = XXWIDGET (xwidget);
> +  CHECK_WEBKIT_WIDGET (xw);
> +
> +  block_input ();
> +#ifdef USE_GTK
> +  webview = WEBKIT_WEB_VIEW (xw->widget_osr);
> +  value = webkit_web_view_get_estimated_load_progress (webview);
> +#elif defined NS_IMPL_COCOA
> +  value = nsxwidget_webkit_estimated_load_progress (xw);
> +#endif
> +
> +  unblock_input ();
> +
> +  return make_float (value);
> +}
> +
>  DEFUN ("xwidget-webkit-goto-uri",
>         Fxwidget_webkit_goto_uri, Sxwidget_webkit_goto_uri,
>         2, 2, 0,
> @@ -3810,28 +3841,6 @@ DEFUN ("xwidget-webkit-back-forward-list", Fxwidget_webkit_back_forward_list,
>    return list3 (back, here, forward);
>  }
>  
> -DEFUN ("xwidget-webkit-estimated-load-progress",
> -       Fxwidget_webkit_estimated_load_progress, Sxwidget_webkit_estimated_load_progress,
> -       1, 1, 0, doc: /* Get the estimated load progress of XWIDGET, a WebKit widget.
> -Return a value ranging from 0.0 to 1.0, based on how close XWIDGET
> -is to completely loading its page.  */)
> -  (Lisp_Object xwidget)
> -{
> -  struct xwidget *xw;
> -  WebKitWebView *webview;
> -  double value;
> -
> -  CHECK_LIVE_XWIDGET (xwidget);
> -  xw = XXWIDGET (xwidget);
> -  CHECK_WEBKIT_WIDGET (xw);
> -
> -  block_input ();
> -  webview = WEBKIT_WEB_VIEW (xw->widget_osr);
> -  value = webkit_web_view_get_estimated_load_progress (webview);
> -  unblock_input ();
> -
> -  return make_float (value);
> -}
>  #endif
>  
>  DEFUN ("xwidget-webkit-set-cookie-storage-file",
> @@ -3874,19 +3883,23 @@ DEFUN ("xwidget-webkit-stop-loading", Fxwidget_webkit_stop_loading,
>  XWIDGET as part of loading a page.  */)
>    (Lisp_Object xwidget)
>  {
> -#ifdef USE_GTK
>    struct xwidget *xw;
> +#ifdef USE_GTK
>    WebKitWebView *webview;
> +#endif
>  
>    CHECK_LIVE_XWIDGET (xwidget);
>    xw = XXWIDGET (xwidget);
>    CHECK_WEBKIT_WIDGET (xw);
>  
>    block_input ();
> +#ifdef USE_GTK
>    webview = WEBKIT_WEB_VIEW (xw->widget_osr);
>    webkit_web_view_stop_loading (webview);
> -  unblock_input ();
> +#elif defined NS_IMPL_COCOA
> +  nsxwidget_webkit_stop_loading (xw);
>  #endif
> +  unblock_input ();
>  
>    return Qnil;
>  }
> @@ -3936,8 +3949,9 @@ syms_of_xwidget (void)
>  #ifdef USE_GTK
>    defsubr (&Sxwidget_webkit_load_html);
>    defsubr (&Sxwidget_webkit_back_forward_list);
> -  defsubr (&Sxwidget_webkit_estimated_load_progress);
>  #endif
> +
> +  defsubr (&Sxwidget_webkit_estimated_load_progress);
>    defsubr (&Skill_xwidget);
>  
>    DEFSYM (QCxwidget, ":xwidget");

None of this seems to affect anything other than the NS build, so it's
fine by me.

All in all, all your changes are fine for Emacs 29 by me.  What they
need is a proper commit message.  See the node "Style of Change Logs" in
the GNU Coding Standards for examples of how to do this:

  https://www.gnu.org/prep/standards/standards.html#Style-of-Change-Logs

Thanks again for working on Emacs.

bug#60703: Patches to xwidget code

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Andrew De Angelis <bobodeangelis@gmail.com> writes:

> Hello everyone and thanks for all your work!
>
> I have some fixes to the xwidget code that I'd like to contribute.
>
> These should achieve three things 1) fix `xwidget-webkit-current-url'
> in xwidget.el so that it actually displays the current URL 2) fix the
> Objective-C code in nsxwidget.m to eliminate memory leaks 3) implement
> the function `xwidget-webkit-estimated-load-progress' in Objective-C,
> making this functionality available for macOS as well
>
> Regarding 2), I have tested my changes with the Instruments app within
> the XCode dev tools. I was able to test them on two different
> machines: a MacBook Air M2 running macOS Ventura 13, and an Intel
> processor running macOS Big Sur 11.6.7.  When testing with
> Instruments, it appears that Emacs isn't leaking memory anymore. I am
> still seeing some leaks, but according to Instruments, the responsible
> libraries are system libraries, and not Emacs itself.  If possible, I
> would appreciate it if other people can test/profile these changes as
> well, and share their feedback.
>
> Assuming the changes are correct and accepted, would it be possible to
> include them in the upcoming Emacs 29.1 release?  Technically, none of
> these are new features; it's just fixes to existing features.
>
> I intend to keep working on xwidget support for macOS (fixing
> remaining issues and implementing missing functionalities), but I'm
> not sure when I'll be able to get to it; for now I'd like to start by
> contributing these first major fixes.
>
> Thanks in advance for your feedback; let me know if you need anything
> else from me.

Andrew, thanks for working on Emacs.

>
> diff --git a/lisp/xwidget.el b/lisp/xwidget.el
> index abbda29081..8095fa9db5 100644
> --- a/lisp/xwidget.el
> +++ b/lisp/xwidget.el
> @@ -924,8 +924,9 @@ xwidget-webkit-reload
>  (defun xwidget-webkit-current-url ()
>    "Display the current xwidget webkit URL and place it on the `kill-ring'."
>    (interactive nil xwidget-webkit-mode)
> -  (let ((url (xwidget-webkit-uri (xwidget-webkit-current-session))))
> -    (message "URL: %s" (kill-new (or url "")))))
> +  (let ((url (or (xwidget-webkit-uri (xwidget-webkit-current-session)) "")))
> +    (kill-new url)
> +    (message "URL: %s" url)))
>  
>  (defun xwidget-webkit-browse-history ()
>    "Display a buffer containing the history of page loads."

This change is fine for Emacs 29.1.  Eli, WDYT?

>  @implementation XwWebView : WKWebView
>  
> +- (void)dealloc
> +{
> +  [super dealloc];
> +}

Thanks.  Please put a space between the part of the Objective-C method
that looks like a cast (I don't know what it's called, sorry) and the
name of the method.

But, is this really necessary?  Without this being defined, the runtime
will just call the super method, correct?

>  - (id)initWithFrame:(CGRect)frame
>        configuration:(WKWebViewConfiguration *)configuration
>              xwidget:(struct xwidget *)xw
>  {
>    /* Script controller to add script message handler and user script.  */
> -  WKUserContentController *scriptor = [[WKUserContentController alloc] init];
> +  WKUserContentController *scriptor = [[[WKUserContentController alloc] init]
> +                                        autorelease];
>    configuration.userContentController = scriptor;
>  
>    /* Enable inspect element context menu item for debugging.  */
> @@ -81,7 +87,8 @@ - (id)initWithFrame:(CGRect)frame
>    if (self)
>      {
>        self.xw = xw;
> -      self.urlScriptBlocked = [[NSMutableDictionary alloc] init];
> +      self.urlScriptBlocked = [[[NSMutableDictionary alloc] init]
> +                                autorelease];
>        self.navigationDelegate = self;
>        self.UIDelegate = self;
>        self.customUserAgent =
> @@ -89,11 +96,13 @@ - (id)initWithFrame:(CGRect)frame
>          @" AppleWebKit/603.3.8 (KHTML, like Gecko)"
>          @" Version/11.0.1 Safari/603.3.8";
>        [scriptor addScriptMessageHandler:self name:@"keyDown"];
> -      [scriptor addUserScript:[[WKUserScript alloc]
> -                                initWithSource:xwScript
> -                                 injectionTime:
> -                                  WKUserScriptInjectionTimeAtDocumentStart
> -                                forMainFrameOnly:NO]];
> +      WKUserScript *userScript = [[[WKUserScript alloc]
> +                                    initWithSource:xwScript
> +                                     injectionTime:
> +                                      WKUserScriptInjectionTimeAtDocumentStart
> +                                    forMainFrameOnly:NO]
> +                                   autorelease];
> +      [scriptor addUserScript:userScript];
>      }
>    return self;
>  }
> @@ -102,7 +111,27 @@ - (void)webView:(WKWebView *)webView
>  didFinishNavigation:(WKNavigation *)navigation
>  {
>    if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
> -    store_xwidget_event_string (self.xw, "load-changed", "");
> +    store_xwidget_event_string (self.xw, "load-changed", "load-finished");
> +}
> +
> +- (void)webView:(WKWebView *)webView didStartProvisionalNavigation:(WKNavigation *)navigation
> +{
> +  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
> +    store_xwidget_event_string (self.xw, "load-changed", "load-started");
> +}
> +
> +- (void)webView:(WKWebView *)webView
> +didReceiveServerRedirectForProvisionalNavigation:(WKNavigation *)navigation
> +{
> +  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
> +    store_xwidget_event_string (self.xw, "load-changed", "load-redirected");
> +}
> +
> +// Start loading WKWebView
> +- (void)webView:(WKWebView *)webView didCommitNavigation:(WKNavigation *)navigation
> +{
> +  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt)) // what exactly is this test for
> +    store_xwidget_event_string (self.xw, "load-changed", "load-committed");
>  }

These are also fine for Emacs 29.  However, please replace the C++-style
comments with C style ones, add spaces after the cast-like parts in the
method definition, and fill everything to column 80.  I believe the test
is that the xwidget's buffer has not been killed, and could easily be
written:

  !NILP (Fbuffer_live_p (self.xw->buffer))

as well.

>  - (void)webView:(WKWebView *)webView
> @@ -343,6 +372,20 @@ - (void)userContentController:(WKUserContentController *)userContentController
>    }
>  }
>  
> +double
> +nsxwidget_webkit_estimated_load_progress(struct xwidget *xw)
> +{
> +  XwWebView *xwWebView = (XwWebView *) xw->xwWidget;
> +  return xwWebView.estimatedProgress;
> +}

Please place a space between "nsxwidget_webkit_estimated_load_progress"
and the parameter list.

> +void
> +nsxwidget_webkit_stop_loading (struct xwidget *xw)
> +{
> +  XwWebView *xwWebView = (XwWebView *) xw->xwWidget;
> +  [xwWebView stopLoading];
> +}
> +
>  void
>  nsxwidget_webkit_zoom (struct xwidget *xw, double zoom_change)
>  {
> @@ -452,7 +495,8 @@ - (BOOL)isFlipped { return YES; }
>    NSRect rect = NSMakeRect (0, 0, xw->width, xw->height);
>    xw->xwWidget = [[XwWebView alloc]
>                     initWithFrame:rect
> -                   configuration:[[WKWebViewConfiguration alloc] init]
> +                   configuration:[[[WKWebViewConfiguration alloc] init]
> +                                   autorelease]
>                           xwidget:xw];
>    xw->xwWindow = [[XwWindow alloc]
>                     initWithFrame:rect];
> @@ -470,16 +514,18 @@ - (BOOL)isFlipped { return YES; }
>          ((XwWebView *) xw->xwWidget).configuration.userContentController;
>        [scriptor removeAllUserScripts];
>        [scriptor removeScriptMessageHandlerForName:@"keyDown"];
> -      [scriptor release];
> +
>        if (xw->xv)
>          xw->xv->model = Qnil; /* Make sure related view stale.  */
>  
>        /* This stops playing audio when a xwidget-webkit buffer is
> -         killed.  I could not find other solution.  */
> +         killed.  I could not find other solution.
> +         TODO: improve this */
>        nsxwidget_webkit_goto_uri (xw, "about:blank");
>  
>        [((XwWebView *) xw->xwWidget).urlScriptBlocked release];
>        [xw->xwWidget removeFromSuperviewWithoutNeedingDisplay];
> +
>        [xw->xwWidget release];
>        [xw->xwWindow removeFromSuperviewWithoutNeedingDisplay];
>        [xw->xwWindow release];

These changes are also fine for Emacs 29, but the TODO seems excessive.
Eli, any comments here?

> diff --git a/src/xwidget.c b/src/xwidget.c
> index efe2705562..b9550da460 100644
> --- a/src/xwidget.c
> +++ b/src/xwidget.c
> @@ -54,6 +54,7 @@ Copyright (C) 2011-2023 Free Software Foundation, Inc.
>  
>  #include <math.h>
>  
> +

Please undo this extraneous whitespace change.

>  
> +DEFUN ("xwidget-webkit-estimated-load-progress",
> +       Fxwidget_webkit_estimated_load_progress, Sxwidget_webkit_estimated_load_progress,
> +       1, 1, 0, doc: /* Get the estimated load progress of XWIDGET, a WebKit widget.
> +Return a value ranging from 0.0 to 1.0, based on how close XWIDGET
> +is to completely loading its page.  */)
> +  (Lisp_Object xwidget)
> +{
> +  struct xwidget *xw;
> +#ifdef USE_GTK
> +  WebKitWebView *webview;
> +#endif
> +  double value;
> +
> +  CHECK_LIVE_XWIDGET (xwidget);
> +  xw = XXWIDGET (xwidget);
> +  CHECK_WEBKIT_WIDGET (xw);
> +
> +  block_input ();
> +#ifdef USE_GTK
> +  webview = WEBKIT_WEB_VIEW (xw->widget_osr);
> +  value = webkit_web_view_get_estimated_load_progress (webview);
> +#elif defined NS_IMPL_COCOA
> +  value = nsxwidget_webkit_estimated_load_progress (xw);
> +#endif
> +
> +  unblock_input ();
> +
> +  return make_float (value);
> +}
> +
>  DEFUN ("xwidget-webkit-goto-uri",
>         Fxwidget_webkit_goto_uri, Sxwidget_webkit_goto_uri,
>         2, 2, 0,
> @@ -3810,28 +3841,6 @@ DEFUN ("xwidget-webkit-back-forward-list", Fxwidget_webkit_back_forward_list,
>    return list3 (back, here, forward);
>  }
>  
> -DEFUN ("xwidget-webkit-estimated-load-progress",
> -       Fxwidget_webkit_estimated_load_progress, Sxwidget_webkit_estimated_load_progress,
> -       1, 1, 0, doc: /* Get the estimated load progress of XWIDGET, a WebKit widget.
> -Return a value ranging from 0.0 to 1.0, based on how close XWIDGET
> -is to completely loading its page.  */)
> -  (Lisp_Object xwidget)
> -{
> -  struct xwidget *xw;
> -  WebKitWebView *webview;
> -  double value;
> -
> -  CHECK_LIVE_XWIDGET (xwidget);
> -  xw = XXWIDGET (xwidget);
> -  CHECK_WEBKIT_WIDGET (xw);
> -
> -  block_input ();
> -  webview = WEBKIT_WEB_VIEW (xw->widget_osr);
> -  value = webkit_web_view_get_estimated_load_progress (webview);
> -  unblock_input ();
> -
> -  return make_float (value);
> -}
>  #endif
>  
>  DEFUN ("xwidget-webkit-set-cookie-storage-file",
> @@ -3874,19 +3883,23 @@ DEFUN ("xwidget-webkit-stop-loading", Fxwidget_webkit_stop_loading,
>  XWIDGET as part of loading a page.  */)
>    (Lisp_Object xwidget)
>  {
> -#ifdef USE_GTK
>    struct xwidget *xw;
> +#ifdef USE_GTK
>    WebKitWebView *webview;
> +#endif
>  
>    CHECK_LIVE_XWIDGET (xwidget);
>    xw = XXWIDGET (xwidget);
>    CHECK_WEBKIT_WIDGET (xw);
>  
>    block_input ();
> +#ifdef USE_GTK
>    webview = WEBKIT_WEB_VIEW (xw->widget_osr);
>    webkit_web_view_stop_loading (webview);
> -  unblock_input ();
> +#elif defined NS_IMPL_COCOA
> +  nsxwidget_webkit_stop_loading (xw);
>  #endif
> +  unblock_input ();
>  
>    return Qnil;
>  }
> @@ -3936,8 +3949,9 @@ syms_of_xwidget (void)
>  #ifdef USE_GTK
>    defsubr (&Sxwidget_webkit_load_html);
>    defsubr (&Sxwidget_webkit_back_forward_list);
> -  defsubr (&Sxwidget_webkit_estimated_load_progress);
>  #endif
> +
> +  defsubr (&Sxwidget_webkit_estimated_load_progress);
>    defsubr (&Skill_xwidget);
>  
>    DEFSYM (QCxwidget, ":xwidget");

None of this seems to affect anything other than the NS build, so it's
fine by me.

All in all, all your changes are fine for Emacs 29 by me.  What they
need is a proper commit message.  See the node "Style of Change Logs" in
the GNU Coding Standards for examples of how to do this:

  https://www.gnu.org/prep/standards/standards.html#Style-of-Change-Logs

Thanks again for working on Emacs.

bug#60703: Patches to xwidget code

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org,  60703@debbugs.gnu.org
> Date: Tue, 10 Jan 2023 17:59:04 +0800
> 
> > --- a/lisp/xwidget.el
> > +++ b/lisp/xwidget.el
> > @@ -924,8 +924,9 @@ xwidget-webkit-reload
> >  (defun xwidget-webkit-current-url ()
> >    "Display the current xwidget webkit URL and place it on the `kill-ring'."
> >    (interactive nil xwidget-webkit-mode)
> > -  (let ((url (xwidget-webkit-uri (xwidget-webkit-current-session))))
> > -    (message "URL: %s" (kill-new (or url "")))))
> > +  (let ((url (or (xwidget-webkit-uri (xwidget-webkit-current-session)) "")))
> > +    (kill-new url)
> > +    (message "URL: %s" url)))
> >  
> >  (defun xwidget-webkit-browse-history ()
> >    "Display a buffer containing the history of page loads."
> 
> This change is fine for Emacs 29.1.  Eli, WDYT?

Fine by me.

> These changes are also fine for Emacs 29, but the TODO seems excessive.
> Eli, any comments here?

I don't have an opinion, so if it's OK in your opinion, I'm fine with
adding this.  Are we sure this will not affect any important aspects
of the build, except where we already have problems?  (I don't see any
description of the problems and their solution in the commit log or
the comments.)

> All in all, all your changes are fine for Emacs 29 by me.  What they
> need is a proper commit message.  See the node "Style of Change Logs" in
> the GNU Coding Standards for examples of how to do this:
> 
>   https://www.gnu.org/prep/standards/standards.html#Style-of-Change-Logs

I don't see copyright assignment for Andrew, so this should be settled
before accepting the contribution of this magnitude.

> Thanks again for working on Emacs.

Seconded.

P.S. Please don't cross-post to emacs-devel.

bug#60703: Patches to xwidget code

[Andrew De Angelis]

[-- Attachment #1: Type: text/plain, Size: 3127 bytes --]

Thank you both.

@Po Lu <luangruo@yahoo.com>, I will work on the changes you mentioned.
Can you clarify the C++-style comments vs C style ones? When should we use
"/* */" and when should we use "//"?
I looked around the code and it doesn't seem very consistent, but I can
help clean up the xwidget-related files.

@Eli,

> Are we sure this will not affect any important aspects
> of the build, except where we already have problems?  (I don't see any
> description of the problems and their solution in the commit log or
> the comments.)
>
This should only affect builds on Mac machines, and makes it so the xwidget
feature doesn't leak memory anymore. Importantly, I do not have access to
any Linux machine, so I can't be 100% sure they're not affected. But as far
as I can tell, the changes are limited to Mac builds using the
"-with-xwidgets" feature.
I'll add a better explanation in the upcoming Change Log, but essentially,
the problems were simply memory leaks. The leaked memory wasn't a lot, so
it seldom caused noticeable issues (at least in my experience).

I'm sending my answers to the copyright questionnaire to assign@gnu.org.
This is my first contribution, so let me know if I'm missing anything.

Best,
Andrew



On Tue, Jan 10, 2023 at 8:40 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Po Lu <luangruo@yahoo.com>
> > Cc: emacs-devel@gnu.org,  60703@debbugs.gnu.org
> > Date: Tue, 10 Jan 2023 17:59:04 +0800
> >
> > > --- a/lisp/xwidget.el
> > > +++ b/lisp/xwidget.el
> > > @@ -924,8 +924,9 @@ xwidget-webkit-reload
> > >  (defun xwidget-webkit-current-url ()
> > >    "Display the current xwidget webkit URL and place it on the
> `kill-ring'."
> > >    (interactive nil xwidget-webkit-mode)
> > > -  (let ((url (xwidget-webkit-uri (xwidget-webkit-current-session))))
> > > -    (message "URL: %s" (kill-new (or url "")))))
> > > +  (let ((url (or (xwidget-webkit-uri
> (xwidget-webkit-current-session)) "")))
> > > +    (kill-new url)
> > > +    (message "URL: %s" url)))
> > >
> > >  (defun xwidget-webkit-browse-history ()
> > >    "Display a buffer containing the history of page loads."
> >
> > This change is fine for Emacs 29.1.  Eli, WDYT?
>
> Fine by me.
>
> > These changes are also fine for Emacs 29, but the TODO seems excessive.
> > Eli, any comments here?
>
> I don't have an opinion, so if it's OK in your opinion, I'm fine with
> adding this.  Are we sure this will not affect any important aspects
> of the build, except where we already have problems?  (I don't see any
> description of the problems and their solution in the commit log or
> the comments.)
>
> > All in all, all your changes are fine for Emacs 29 by me.  What they
> > need is a proper commit message.  See the node "Style of Change Logs" in
> > the GNU Coding Standards for examples of how to do this:
> >
> >   https://www.gnu.org/prep/standards/standards.html#Style-of-Change-Logs
>
> I don't see copyright assignment for Andrew, so this should be settled
> before accepting the contribution of this magnitude.
>
> > Thanks again for working on Emacs.
>
> Seconded.
>
> P.S. Please don't cross-post to emacs-devel.
>

[-- Attachment #2: Type: text/html, Size: 4944 bytes --]

bug#60703: Patches to xwidget code

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> I don't have an opinion, so if it's OK in your opinion, I'm fine with
> adding this.  Are we sure this will not affect any important aspects
> of the build, except where we already have problems?  (I don't see any
> description of the problems and their solution in the commit log or
> the comments.)

Yes, these are fixes to a piece of code that is normally off by default,
and did not work prior to those changes.

> I don't see copyright assignment for Andrew, so this should be settled
> before accepting the contribution of this magnitude.

Right, thanks for bringing that up.

bug#60703: Patches to xwidget code

[Andrew De Angelis]

[-- Attachment #1.1: Type: text/plain, Size: 1280 bytes --]

Thank you again for the feedback. Attached is the updated patch.
@Po Lu <luangruo@yahoo.com>, you were right about the dealloc method: I
removed it since it was unnecessary.
I think I fixed all the issues you pointed out, but let me know if there's
any other changes needed.

About the copyright assignment, I sent the attached email to assign@gnu.org
ten days ago, but haven't gotten a reply yet. Just want to make sure I'm
going about it the right way: do I need to do anything else to get this
done? It's my first contribution so I may be missing something.


On Tue, Jan 10, 2023 at 8:15 PM Po Lu <luangruo@yahoo.com> wrote:

> Eli Zaretskii <eliz@gnu.org> writes:
>
> > I don't have an opinion, so if it's OK in your opinion, I'm fine with
> > adding this.  Are we sure this will not affect any important aspects
> > of the build, except where we already have problems?  (I don't see any
> > description of the problems and their solution in the commit log or
> > the comments.)
>
> Yes, these are fixes to a piece of code that is normally off by default,
> and did not work prior to those changes.
>
> > I don't see copyright assignment for Andrew, so this should be settled
> > before accepting the contribution of this magnitude.
>
> Right, thanks for bringing that up.
>

[-- Attachment #1.2: Type: text/html, Size: 1988 bytes --]

[-- Attachment #2: Andrew John De Angelis.eml --]
[-- Type: message/rfc822, Size: 2714 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 882 bytes --]

----------------------------------------------------------------------
REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES

[What is the name of the program or package you're contributing to?]
Emacs

[Did you copy any files or text written by someone else in these changes?
Even if that material is free software, we need to know about it.]
No

[Do you have an employer who might have a basis to claim to own
your changes?  Do you attend a school which might make such a claim?]
No

[For the copyright registration, what country are you a citizen of?]
USA

[What year were you born?]
1995

[Please write your email address here.]
bobodeangelis@gmail.com

[Please write your postal address here.]
1239 Pacific St, 1F, Brooklyn, NY 11216





[Which files have you changed so far, and which new files have you written
so far?]
lisp/xwidget.el
src/nsxwidget.h
src/nsxwidget.m
src/xwidget.c

[-- Attachment #2.1.2: Type: text/html, Size: 1287 bytes --]

[-- Attachment #3: xwidget-patches-1.patch --]
[-- Type: application/octet-stream, Size: 14123 bytes --]

diff --git a/lisp/xwidget.el b/lisp/xwidget.el
index abbda29081..8095fa9db5 100644
--- a/lisp/xwidget.el
+++ b/lisp/xwidget.el
@@ -924,8 +924,9 @@ xwidget-webkit-reload
 (defun xwidget-webkit-current-url ()
   "Display the current xwidget webkit URL and place it on the `kill-ring'."
   (interactive nil xwidget-webkit-mode)
-  (let ((url (xwidget-webkit-uri (xwidget-webkit-current-session))))
-    (message "URL: %s" (kill-new (or url "")))))
+  (let ((url (or (xwidget-webkit-uri (xwidget-webkit-current-session)) "")))
+    (kill-new url)
+    (message "URL: %s" url)))
 
 (defun xwidget-webkit-browse-history ()
   "Display a buffer containing the history of page loads."
diff --git a/src/nsxwidget.h b/src/nsxwidget.h
index 8d55fac532..2b5596f905 100644
--- a/src/nsxwidget.h
+++ b/src/nsxwidget.h
@@ -36,6 +36,8 @@ #define NSXWIDGET_H_INCLUDED
 Lisp_Object nsxwidget_webkit_title (struct xwidget *xw);
 void nsxwidget_webkit_goto_uri (struct xwidget *xw, const char *uri);
 void nsxwidget_webkit_goto_history (struct xwidget *xw, int rel_pos);
+double nsxwidget_webkit_estimated_load_progress(struct xwidget *xw);
+void nsxwidget_webkit_stop_loading (struct xwidget *xw);
 void nsxwidget_webkit_zoom (struct xwidget *xw, double zoom_change);
 void nsxwidget_webkit_execute_script (struct xwidget *xw, const char *script,
                                       Lisp_Object fun);
diff --git a/src/nsxwidget.m b/src/nsxwidget.m
index e1fbd749b6..a505ad0e2c 100644
--- a/src/nsxwidget.m
+++ b/src/nsxwidget.m
@@ -57,12 +57,13 @@ @interface XwWebView : WKWebView
 @end
 @implementation XwWebView : WKWebView
 
-- (id)initWithFrame:(CGRect)frame
+- (id) initWithFrame:(CGRect)frame
       configuration:(WKWebViewConfiguration *)configuration
             xwidget:(struct xwidget *)xw
 {
   /* Script controller to add script message handler and user script.  */
-  WKUserContentController *scriptor = [[WKUserContentController alloc] init];
+  WKUserContentController *scriptor = [[[WKUserContentController alloc] init]
+                                        autorelease];
   configuration.userContentController = scriptor;
 
   /* Enable inspect element context menu item for debugging.  */
@@ -81,7 +82,8 @@ - (id)initWithFrame:(CGRect)frame
   if (self)
     {
       self.xw = xw;
-      self.urlScriptBlocked = [[NSMutableDictionary alloc] init];
+      self.urlScriptBlocked = [[[NSMutableDictionary alloc] init]
+                                autorelease];
       self.navigationDelegate = self;
       self.UIDelegate = self;
       self.customUserAgent =
@@ -89,23 +91,45 @@ - (id)initWithFrame:(CGRect)frame
         @" AppleWebKit/603.3.8 (KHTML, like Gecko)"
         @" Version/11.0.1 Safari/603.3.8";
       [scriptor addScriptMessageHandler:self name:@"keyDown"];
-      [scriptor addUserScript:[[WKUserScript alloc]
-                                initWithSource:xwScript
-                                 injectionTime:
-                                  WKUserScriptInjectionTimeAtDocumentStart
-                                forMainFrameOnly:NO]];
+      WKUserScript *userScript = [[[WKUserScript alloc]
+                                    initWithSource:xwScript
+                                     injectionTime:
+                                      WKUserScriptInjectionTimeAtDocumentStart
+                                    forMainFrameOnly:NO] autorelease];
+      [scriptor addUserScript:userScript];
     }
   return self;
 }
 
-- (void)webView:(WKWebView *)webView
+- (void) webView:(WKWebView *)webView
 didFinishNavigation:(WKNavigation *)navigation
 {
   if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
-    store_xwidget_event_string (self.xw, "load-changed", "");
+    store_xwidget_event_string (self.xw, "load-changed", "load-finished");
 }
 
-- (void)webView:(WKWebView *)webView
+- (void) webView:(WKWebView *)webView didStartProvisionalNavigation:(WKNavigation *)navigation
+{
+  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
+    store_xwidget_event_string (self.xw, "load-changed", "load-started");
+}
+
+- (void) webView:(WKWebView *)webView
+didReceiveServerRedirectForProvisionalNavigation:(WKNavigation *)navigation
+{
+  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
+    store_xwidget_event_string (self.xw, "load-changed", "load-redirected");
+}
+
+/* Start loading WKWebView */
+- (void) webView:(WKWebView *)webView
+didCommitNavigation:(WKNavigation *)navigation
+{
+  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
+    store_xwidget_event_string (self.xw, "load-changed", "load-committed");
+}
+
+- (void) webView:(WKWebView *)webView
 decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction
 decisionHandler:(void (^)(WKNavigationActionPolicy))decisionHandler
 {
@@ -114,13 +138,13 @@ - (void)webView:(WKWebView *)webView
     decisionHandler (WKNavigationActionPolicyAllow);
     break;
   default:
-    // decisionHandler (WKNavigationActionPolicyCancel);
+    /* decisionHandler (WKNavigationActionPolicyCancel); */
     decisionHandler (WKNavigationActionPolicyAllow);
     break;
   }
 }
 
-- (void)webView:(WKWebView *)webView
+- (void) webView:(WKWebView *)webView
 decidePolicyForNavigationResponse:(WKNavigationResponse *)navigationResponse
 decisionHandler:(void (^)(WKNavigationResponsePolicy))decisionHandler
 {
@@ -166,7 +190,7 @@ - (void)webView:(WKWebView *)webView
 
 /* No additional new webview or emacs window will be created
    for <a ... target="_blank">.  */
-- (WKWebView *)webView:(WKWebView *)webView
+- (WKWebView *) webView:(WKWebView *)webView
 createWebViewWithConfiguration:(WKWebViewConfiguration *)configuration
    forNavigationAction:(WKNavigationAction *)navigationAction
         windowFeatures:(WKWindowFeatures *)windowFeatures
@@ -177,7 +201,7 @@ - (WKWebView *)webView:(WKWebView *)webView
 }
 
 /* Open panel for file upload.  */
-- (void)webView:(WKWebView *)webView
+- (void) webView:(WKWebView *)webView
 runOpenPanelWithParameters:(WKOpenPanelParameters *)parameters
 initiatedByFrame:(WKFrameInfo *)frame
 completionHandler:(void (^)(NSArray<NSURL *> *URLs))completionHandler
@@ -197,13 +221,13 @@ - (void)webView:(WKWebView *)webView
    - Correct mouse hand/arrow/I-beam is displayed (TODO: not perfect yet).
 */
 
-- (void)mouseDown:(NSEvent *)event
+- (void) mouseDown:(NSEvent *)event
 {
   [self.xw->xv->emacswindow mouseDown:event];
   [super mouseDown:event];
 }
 
-- (void)mouseUp:(NSEvent *)event
+- (void) mouseUp:(NSEvent *)event
 {
   [self.xw->xv->emacswindow mouseUp:event];
   [super mouseUp:event];
@@ -214,7 +238,7 @@ - (void)mouseUp:(NSEvent *)event
    emacs as first responder to avoid focus held in an input element
    with matching text.  */
 
-- (void)keyDown:(NSEvent *)event
+- (void) keyDown:(NSEvent *)event
 {
   Lisp_Object var = Fintern (build_string ("isearch-mode"), Qnil);
   Lisp_Object val = buffer_local_value (var, Fcurrent_buffer ());
@@ -250,7 +274,7 @@ - (void)keyDown:(NSEvent *)event
     }];
 }
 
-- (void)interpretKeyEvents:(NSArray<NSEvent *> *)eventArray
+- (void) interpretKeyEvents:(NSArray<NSEvent *> *)eventArray
 {
   /* We should do nothing and do not forward (default implementation
      if we not override here) to let emacs collect key events and ask
@@ -258,7 +282,7 @@ - (void)interpretKeyEvents:(NSArray<NSEvent *> *)eventArray
 }
 
 static NSString *xwScript;
-+ (void)initialize
++ (void) initialize
 {
   /* Find out if an input element has focus.
      Message to script message handler when 'C-g' key down.  */
@@ -284,7 +308,7 @@ + (void)initialize
 
 /* Confirming to WKScriptMessageHandler, listens concerning keyDown in
    webkit. Currently 'C-g'.  */
-- (void)userContentController:(WKUserContentController *)userContentController
+- (void) userContentController:(WKUserContentController *)userContentController
       didReceiveScriptMessage:(WKScriptMessage *)message
 {
   if ([message.body isEqualToString:@"C-g"])
@@ -343,6 +367,20 @@ - (void)userContentController:(WKUserContentController *)userContentController
   }
 }
 
+double
+nsxwidget_webkit_estimated_load_progress (struct xwidget *xw)
+{
+  XwWebView *xwWebView = (XwWebView *) xw->xwWidget;
+  return xwWebView.estimatedProgress;
+}
+
+void
+nsxwidget_webkit_stop_loading (struct xwidget *xw)
+{
+  XwWebView *xwWebView = (XwWebView *) xw->xwWidget;
+  [xwWebView stopLoading];
+}
+
 void
 nsxwidget_webkit_zoom (struct xwidget *xw, double zoom_change)
 {
@@ -430,7 +468,7 @@ - (void)userContentController:(WKUserContentController *)userContentController
         }
       else if (result && FUNCTIONP (fun))
         {
-          // NSLog (@"result=%@, type=%@", result, [result class]);
+          /* NSLog (@"result=%@, type=%@", result, [result class]); */
           Lisp_Object lisp_value = js_to_lisp (result);
           store_xwidget_js_callback_event (xw, fun, lisp_value);
         }
@@ -440,19 +478,20 @@ - (void)userContentController:(WKUserContentController *)userContentController
 /* Window containing an xwidget.  */
 
 @implementation XwWindow
-- (BOOL)isFlipped { return YES; }
+- (BOOL) isFlipped { return YES; }
 @end
 
 /* Xwidget model, macOS Cocoa part.  */
 
 void
-nsxwidget_init(struct xwidget *xw)
+nsxwidget_init (struct xwidget *xw)
 {
   block_input ();
   NSRect rect = NSMakeRect (0, 0, xw->width, xw->height);
   xw->xwWidget = [[XwWebView alloc]
                    initWithFrame:rect
-                   configuration:[[WKWebViewConfiguration alloc] init]
+                   configuration:[[[WKWebViewConfiguration alloc] init]
+                                   autorelease]
                          xwidget:xw];
   xw->xwWindow = [[XwWindow alloc]
                    initWithFrame:rect];
@@ -470,16 +509,18 @@ - (BOOL)isFlipped { return YES; }
         ((XwWebView *) xw->xwWidget).configuration.userContentController;
       [scriptor removeAllUserScripts];
       [scriptor removeScriptMessageHandlerForName:@"keyDown"];
-      [scriptor release];
+
       if (xw->xv)
         xw->xv->model = Qnil; /* Make sure related view stale.  */
 
       /* This stops playing audio when a xwidget-webkit buffer is
-         killed.  I could not find other solution.  */
+         killed.  I could not find other solution.
+         TODO: improve this */
       nsxwidget_webkit_goto_uri (xw, "about:blank");
 
       [((XwWebView *) xw->xwWidget).urlScriptBlocked release];
       [xw->xwWidget removeFromSuperviewWithoutNeedingDisplay];
+
       [xw->xwWidget release];
       [xw->xwWindow removeFromSuperviewWithoutNeedingDisplay];
       [xw->xwWindow release];
@@ -507,7 +548,7 @@ - (BOOL)isFlipped { return YES; }
 /* Xwidget view, macOS Cocoa part.  */
 
 @implementation XvWindow : NSView
-- (BOOL)isFlipped { return YES; }
+- (BOOL) isFlipped { return YES; }
 @end
 
 void
diff --git a/src/xwidget.c b/src/xwidget.c
index efe2705562..c878063eb7 100644
--- a/src/xwidget.c
+++ b/src/xwidget.c
@@ -490,10 +490,10 @@ DEFUN ("xwidget-perform-lispy-event",
 {
   struct xwidget *xw;
   struct frame *f = NULL;
-  int character = -1, keycode = -1;
-  int modifiers = 0;
 
 #ifdef USE_GTK
+  int character = -1, keycode = -1;
+  int modifiers = 0;
   GdkEvent *xg_event;
   GtkContainerClass *klass;
   GtkWidget *widget;
@@ -3063,6 +3063,36 @@ DEFUN ("xwidget-webkit-title",
 #endif
 }
 
+DEFUN ("xwidget-webkit-estimated-load-progress",
+       Fxwidget_webkit_estimated_load_progress, Sxwidget_webkit_estimated_load_progress,
+       1, 1, 0, doc: /* Get the estimated load progress of XWIDGET, a WebKit widget.
+Return a value ranging from 0.0 to 1.0, based on how close XWIDGET
+is to completely loading its page.  */)
+  (Lisp_Object xwidget)
+{
+  struct xwidget *xw;
+#ifdef USE_GTK
+  WebKitWebView *webview;
+#endif
+  double value;
+
+  CHECK_LIVE_XWIDGET (xwidget);
+  xw = XXWIDGET (xwidget);
+  CHECK_WEBKIT_WIDGET (xw);
+
+  block_input ();
+#ifdef USE_GTK
+  webview = WEBKIT_WEB_VIEW (xw->widget_osr);
+  value = webkit_web_view_get_estimated_load_progress (webview);
+#elif defined NS_IMPL_COCOA
+  value = nsxwidget_webkit_estimated_load_progress (xw);
+#endif
+
+  unblock_input ();
+
+  return make_float (value);
+}
+
 DEFUN ("xwidget-webkit-goto-uri",
        Fxwidget_webkit_goto_uri, Sxwidget_webkit_goto_uri,
        2, 2, 0,
@@ -3810,28 +3840,6 @@ DEFUN ("xwidget-webkit-back-forward-list", Fxwidget_webkit_back_forward_list,
   return list3 (back, here, forward);
 }
 
-DEFUN ("xwidget-webkit-estimated-load-progress",
-       Fxwidget_webkit_estimated_load_progress, Sxwidget_webkit_estimated_load_progress,
-       1, 1, 0, doc: /* Get the estimated load progress of XWIDGET, a WebKit widget.
-Return a value ranging from 0.0 to 1.0, based on how close XWIDGET
-is to completely loading its page.  */)
-  (Lisp_Object xwidget)
-{
-  struct xwidget *xw;
-  WebKitWebView *webview;
-  double value;
-
-  CHECK_LIVE_XWIDGET (xwidget);
-  xw = XXWIDGET (xwidget);
-  CHECK_WEBKIT_WIDGET (xw);
-
-  block_input ();
-  webview = WEBKIT_WEB_VIEW (xw->widget_osr);
-  value = webkit_web_view_get_estimated_load_progress (webview);
-  unblock_input ();
-
-  return make_float (value);
-}
 #endif
 
 DEFUN ("xwidget-webkit-set-cookie-storage-file",
@@ -3874,19 +3882,23 @@ DEFUN ("xwidget-webkit-stop-loading", Fxwidget_webkit_stop_loading,
 XWIDGET as part of loading a page.  */)
   (Lisp_Object xwidget)
 {
-#ifdef USE_GTK
   struct xwidget *xw;
+#ifdef USE_GTK
   WebKitWebView *webview;
+#endif
 
   CHECK_LIVE_XWIDGET (xwidget);
   xw = XXWIDGET (xwidget);
   CHECK_WEBKIT_WIDGET (xw);
 
   block_input ();
+#ifdef USE_GTK
   webview = WEBKIT_WEB_VIEW (xw->widget_osr);
   webkit_web_view_stop_loading (webview);
-  unblock_input ();
+#elif defined NS_IMPL_COCOA
+  nsxwidget_webkit_stop_loading (xw);
 #endif
+  unblock_input ();
 
   return Qnil;
 }
@@ -3936,8 +3948,9 @@ syms_of_xwidget (void)
 #ifdef USE_GTK
   defsubr (&Sxwidget_webkit_load_html);
   defsubr (&Sxwidget_webkit_back_forward_list);
-  defsubr (&Sxwidget_webkit_estimated_load_progress);
 #endif
+
+  defsubr (&Sxwidget_webkit_estimated_load_progress);
   defsubr (&Skill_xwidget);
 
   DEFSYM (QCxwidget, ":xwidget");

bug#60703: Patches to xwidget code

[Eli Zaretskii]

> From: Andrew De Angelis <bobodeangelis@gmail.com>
> Date: Sat, 21 Jan 2023 01:13:01 -0500
> Cc: Eli Zaretskii <eliz@gnu.org>, 60703@debbugs.gnu.org
> 
> About the copyright assignment, I sent the attached email to assign@gnu.org ten days ago, but haven't
> gotten a reply yet.

Are you sure?  I have in my email archives a response to your email
from copyright-clerk@fsf.org that was sent on Dec 28, with the PDF of
your assignment attached; you were supposed to sign it and email the
scan of the signed form to them.  Not sure why you sent another
request on Jan 10.  What am I missing here?

bug#60703: Patches to xwidget code

[Andrew De Angelis]

[-- Attachment #1: Type: text/plain, Size: 1410 bytes --]

Sorry about the confusion!
Looks like I missed that reply.
Some quick clarification though: the first assignment I sent out was for
some modifications to shell.el (bug#60385; see also the thread "New
Package: sticky-shell" in emacs-devel: I had a package idea and it was
mentioned to add it as a feature instead). I then 1) missed the reply from
copyright-clerk@fsf.org, 2) assumed I'd need an additional copyright
assignment for the xwidget-patch, as it concerns different files.
Is the first assignment good enough for all subsequent files I change? If
that's the case I'll go ahead and sign it and send it back asap. Just
wanted to make sure I'm following the correct procedure.

Thanks for walking me through this.

On Sat, Jan 21, 2023 at 2:16 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Andrew De Angelis <bobodeangelis@gmail.com>
> > Date: Sat, 21 Jan 2023 01:13:01 -0500
> > Cc: Eli Zaretskii <eliz@gnu.org>, 60703@debbugs.gnu.org
> >
> > About the copyright assignment, I sent the attached email to
> assign@gnu.org ten days ago, but haven't
> > gotten a reply yet.
>
> Are you sure?  I have in my email archives a response to your email
> from copyright-clerk@fsf.org that was sent on Dec 28, with the PDF of
> your assignment attached; you were supposed to sign it and email the
> scan of the signed form to them.  Not sure why you sent another
> request on Jan 10.  What am I missing here?
>

[-- Attachment #2: Type: text/html, Size: 2192 bytes --]

bug#60703: Patches to xwidget code

[Eli Zaretskii]

> From: Andrew De Angelis <bobodeangelis@gmail.com>
> Date: Sat, 21 Jan 2023 10:32:19 -0500
> Cc: luangruo@yahoo.com, 60703@debbugs.gnu.org
> 
> Sorry about the confusion!
> Looks like I missed that reply.
> Some quick clarification though: the first assignment I sent out was for some modifications to shell.el
> (bug#60385; see also the thread "New Package: sticky-shell" in emacs-devel: I had a package idea and it
> was mentioned to add it as a feature instead). I then 1) missed the reply from copyright-clerk@fsf.org, 2)
> assumed I'd need an additional copyright assignment for the xwidget-patch, as it concerns different files.
> Is the first assignment good enough for all subsequent files I change? If that's the case I'll go ahead and sign
> it and send it back asap. Just wanted to make sure I'm following the correct procedure.

The assignment will be good for all your future contributions to
Emacs, but first you need to do what the copyright clerk asked you to
do: print the PDF form, sign it, scan it, and email it back.  Then
you'll get another email saying that your paperwork is complete, and
then we can accept contributions from you.

Thanks.

bug#60703: Patches to xwidget code

[Andrew De Angelis]

[-- Attachment #1.1: Type: text/plain, Size: 2748 bytes --]

Thanks for the explanation. I sent out the signed copy yesterday.
I noticed a couple things about the previous patch, so I'm sending this one
with a few updates:

   - `xwidget-webkit-current-url` in lisp/xwidget.el: check if the url
   variable is non-nil before calling `kill-new' on it. This avoids killing an
   empty string, which would be pointless. We still alert the user that
   something's wrong by messaging "URL: nil" (although getting a nil url seems
   very unlikely).
   - in src/nsxwidget.m: formatting (keep lines below 80 char), and a brief
   comment describing the purpose of some of the newly added functions


I'm also attaching a draft of the ChangeLog. Let me know if you'd like me
to make any changes there.

I do have a question about the X11/GTK implementations for xwidget. I'm not
sure I understand the relationship between the two: from the preprocessor
macros it seems at times they are separate and at times that one is an
addendum to the other.
I'm asking because I want to make sure I'm using the preprocessor macros
correctly: specifically, the function
`xwidget-webkit-estimated-load-progress` within xwidget.c used to be inside
a "#ifdef USE_GTK" block. I moved it outside of the block, and separated
the GTK implementation from the NS_IMPL_COCOA implementation. Is this a
problem for the X11 build? Should I instead put the whole function within a
block along these lines: "#if defined USE_GTK || defined NS_IMPL_COCOA" ?


On Sat, Jan 21, 2023 at 10:41 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Andrew De Angelis <bobodeangelis@gmail.com>
> > Date: Sat, 21 Jan 2023 10:32:19 -0500
> > Cc: luangruo@yahoo.com, 60703@debbugs.gnu.org
> >
> > Sorry about the confusion!
> > Looks like I missed that reply.
> > Some quick clarification though: the first assignment I sent out was for
> some modifications to shell.el
> > (bug#60385; see also the thread "New Package: sticky-shell" in
> emacs-devel: I had a package idea and it
> > was mentioned to add it as a feature instead). I then 1) missed the
> reply from copyright-clerk@fsf.org, 2)
> > assumed I'd need an additional copyright assignment for the
> xwidget-patch, as it concerns different files.
> > Is the first assignment good enough for all subsequent files I change?
> If that's the case I'll go ahead and sign
> > it and send it back asap. Just wanted to make sure I'm following the
> correct procedure.
>
> The assignment will be good for all your future contributions to
> Emacs, but first you need to do what the copyright clerk asked you to
> do: print the PDF form, sign it, scan it, and email it back.  Then
> you'll get another email saying that your paperwork is complete, and
> then we can accept contributions from you.
>
> Thanks.
>

[-- Attachment #1.2: Type: text/html, Size: 3559 bytes --]

[-- Attachment #2: ChangeLog.andrewda --]
[-- Type: application/octet-stream, Size: 1280 bytes --]

2023-01-26  Andrew De Angelis  <bobodeangelis@gmail.com>
	* src/nsxwidget.m ()
	([XwWebView initWithFrame:configuration:xwidget:])
	(nsxwidget_init):  Fixed memory leaks: when sending an alloc message to
	an object, send an autorelease message to any objects we won't
	explictly release
	([XwWebView webView:didFinishNavigation:]): Second string to store in
	store_xwidget_event_string is "load finished" rather than empty string
	([XwWebView webView:didStartProvisionalNavigation:])
	([XwWebView webView:didReceiveServerRedirectForProvisionalNavigation:])
	([XwWebView webView:didCommitNavigation:]): New functions
	(nsxwidget_webkit_estimated_load_progress): New function
	(nsxwidget_webkit_stop_loading): New function
	* src/xwidget.c (Fxwidget_webkit_estimated_load_progress): Call
	nsxwidget_webkit_estimated_load_progress if we're on MacOS
	(Fxwidget_webkit_stop_loading): Call nsxwidget_webkit_stop_loading if
	we're on MacOS
	(syms_of_xwidget): Define symbol for function
	xwidget_webkit_estimated_load_progress if we're on MacOS
	* src/nsxwidget.h: signature for functions
	nsxwidget_webkit_estimated_load_progress and
	nsxwidget_webkit_stop_loading
	* lisp/xwidget.el (xwidget-webkit-current-url): Message URL rather than
	return value of kill-new (which is always nil)

[-- Attachment #3: xwidget-patches-2.patch --]
[-- Type: application/octet-stream, Size: 13807 bytes --]

diff --git a/lisp/xwidget.el b/lisp/xwidget.el
index abbda29081..7daca81f9f 100644
--- a/lisp/xwidget.el
+++ b/lisp/xwidget.el
@@ -925,7 +925,8 @@ xwidget-webkit-current-url
   "Display the current xwidget webkit URL and place it on the `kill-ring'."
   (interactive nil xwidget-webkit-mode)
   (let ((url (xwidget-webkit-uri (xwidget-webkit-current-session))))
-    (message "URL: %s" (kill-new (or url "")))))
+    (when url (kill-new url))
+    (message "URL: %s" url)))
 
 (defun xwidget-webkit-browse-history ()
   "Display a buffer containing the history of page loads."
diff --git a/src/nsxwidget.h b/src/nsxwidget.h
index 8d55fac532..2b5596f905 100644
--- a/src/nsxwidget.h
+++ b/src/nsxwidget.h
@@ -36,6 +36,8 @@ #define NSXWIDGET_H_INCLUDED
 Lisp_Object nsxwidget_webkit_title (struct xwidget *xw);
 void nsxwidget_webkit_goto_uri (struct xwidget *xw, const char *uri);
 void nsxwidget_webkit_goto_history (struct xwidget *xw, int rel_pos);
+double nsxwidget_webkit_estimated_load_progress(struct xwidget *xw);
+void nsxwidget_webkit_stop_loading (struct xwidget *xw);
 void nsxwidget_webkit_zoom (struct xwidget *xw, double zoom_change);
 void nsxwidget_webkit_execute_script (struct xwidget *xw, const char *script,
                                       Lisp_Object fun);
diff --git a/src/nsxwidget.m b/src/nsxwidget.m
index e1fbd749b6..0e00589bb7 100644
--- a/src/nsxwidget.m
+++ b/src/nsxwidget.m
@@ -57,12 +57,13 @@ @interface XwWebView : WKWebView
 @end
 @implementation XwWebView : WKWebView
 
-- (id)initWithFrame:(CGRect)frame
+- (id) initWithFrame:(CGRect)frame
       configuration:(WKWebViewConfiguration *)configuration
             xwidget:(struct xwidget *)xw
 {
   /* Script controller to add script message handler and user script.  */
-  WKUserContentController *scriptor = [[WKUserContentController alloc] init];
+  WKUserContentController *scriptor = [[[WKUserContentController alloc] init]
+                                        autorelease];
   configuration.userContentController = scriptor;
 
   /* Enable inspect element context menu item for debugging.  */
@@ -81,7 +82,8 @@ - (id)initWithFrame:(CGRect)frame
   if (self)
     {
       self.xw = xw;
-      self.urlScriptBlocked = [[NSMutableDictionary alloc] init];
+      self.urlScriptBlocked = [[[NSMutableDictionary alloc] init]
+                                autorelease];
       self.navigationDelegate = self;
       self.UIDelegate = self;
       self.customUserAgent =
@@ -89,23 +91,48 @@ - (id)initWithFrame:(CGRect)frame
         @" AppleWebKit/603.3.8 (KHTML, like Gecko)"
         @" Version/11.0.1 Safari/603.3.8";
       [scriptor addScriptMessageHandler:self name:@"keyDown"];
-      [scriptor addUserScript:[[WKUserScript alloc]
-                                initWithSource:xwScript
-                                 injectionTime:
-                                  WKUserScriptInjectionTimeAtDocumentStart
-                                forMainFrameOnly:NO]];
+      WKUserScript *userScript = [[[WKUserScript alloc]
+                                    initWithSource:xwScript
+                                     injectionTime:
+                                      WKUserScriptInjectionTimeAtDocumentStart
+                                    forMainFrameOnly:NO] autorelease];
+      [scriptor addUserScript:userScript];
     }
   return self;
 }
 
-- (void)webView:(WKWebView *)webView
+/* These 4 functions emulate the behavior of webkit_view_load_changed_cb
+   in the GTK implementation*/
+- (void) webView:(WKWebView *)webView
 didFinishNavigation:(WKNavigation *)navigation
 {
   if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
-    store_xwidget_event_string (self.xw, "load-changed", "");
+    store_xwidget_event_string (self.xw, "load-changed", "load-finished");
 }
 
-- (void)webView:(WKWebView *)webView
+- (void) webView:(WKWebView *)webView
+didStartProvisionalNavigation:(WKNavigation *)navigation
+{
+  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
+    store_xwidget_event_string (self.xw, "load-changed", "load-started");
+}
+
+- (void) webView:(WKWebView *)webView
+didReceiveServerRedirectForProvisionalNavigation:(WKNavigation *)navigation
+{
+  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
+    store_xwidget_event_string (self.xw, "load-changed", "load-redirected");
+}
+
+/* Start loading WKWebView */
+- (void) webView:(WKWebView *)webView
+didCommitNavigation:(WKNavigation *)navigation
+{
+  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
+    store_xwidget_event_string (self.xw, "load-changed", "load-committed");
+}
+
+- (void) webView:(WKWebView *)webView
 decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction
 decisionHandler:(void (^)(WKNavigationActionPolicy))decisionHandler
 {
@@ -114,13 +141,13 @@ - (void)webView:(WKWebView *)webView
     decisionHandler (WKNavigationActionPolicyAllow);
     break;
   default:
-    // decisionHandler (WKNavigationActionPolicyCancel);
+    /* decisionHandler (WKNavigationActionPolicyCancel); */
     decisionHandler (WKNavigationActionPolicyAllow);
     break;
   }
 }
 
-- (void)webView:(WKWebView *)webView
+- (void) webView:(WKWebView *)webView
 decidePolicyForNavigationResponse:(WKNavigationResponse *)navigationResponse
 decisionHandler:(void (^)(WKNavigationResponsePolicy))decisionHandler
 {
@@ -166,7 +193,7 @@ - (void)webView:(WKWebView *)webView
 
 /* No additional new webview or emacs window will be created
    for <a ... target="_blank">.  */
-- (WKWebView *)webView:(WKWebView *)webView
+- (WKWebView *) webView:(WKWebView *)webView
 createWebViewWithConfiguration:(WKWebViewConfiguration *)configuration
    forNavigationAction:(WKNavigationAction *)navigationAction
         windowFeatures:(WKWindowFeatures *)windowFeatures
@@ -177,7 +204,7 @@ - (WKWebView *)webView:(WKWebView *)webView
 }
 
 /* Open panel for file upload.  */
-- (void)webView:(WKWebView *)webView
+- (void) webView:(WKWebView *)webView
 runOpenPanelWithParameters:(WKOpenPanelParameters *)parameters
 initiatedByFrame:(WKFrameInfo *)frame
 completionHandler:(void (^)(NSArray<NSURL *> *URLs))completionHandler
@@ -197,13 +224,13 @@ - (void)webView:(WKWebView *)webView
    - Correct mouse hand/arrow/I-beam is displayed (TODO: not perfect yet).
 */
 
-- (void)mouseDown:(NSEvent *)event
+- (void) mouseDown:(NSEvent *)event
 {
   [self.xw->xv->emacswindow mouseDown:event];
   [super mouseDown:event];
 }
 
-- (void)mouseUp:(NSEvent *)event
+- (void) mouseUp:(NSEvent *)event
 {
   [self.xw->xv->emacswindow mouseUp:event];
   [super mouseUp:event];
@@ -214,7 +241,7 @@ - (void)mouseUp:(NSEvent *)event
    emacs as first responder to avoid focus held in an input element
    with matching text.  */
 
-- (void)keyDown:(NSEvent *)event
+- (void) keyDown:(NSEvent *)event
 {
   Lisp_Object var = Fintern (build_string ("isearch-mode"), Qnil);
   Lisp_Object val = buffer_local_value (var, Fcurrent_buffer ());
@@ -250,7 +277,7 @@ - (void)keyDown:(NSEvent *)event
     }];
 }
 
-- (void)interpretKeyEvents:(NSArray<NSEvent *> *)eventArray
+- (void) interpretKeyEvents:(NSArray<NSEvent *> *)eventArray
 {
   /* We should do nothing and do not forward (default implementation
      if we not override here) to let emacs collect key events and ask
@@ -258,7 +285,7 @@ - (void)interpretKeyEvents:(NSArray<NSEvent *> *)eventArray
 }
 
 static NSString *xwScript;
-+ (void)initialize
++ (void) initialize
 {
   /* Find out if an input element has focus.
      Message to script message handler when 'C-g' key down.  */
@@ -284,7 +311,7 @@ + (void)initialize
 
 /* Confirming to WKScriptMessageHandler, listens concerning keyDown in
    webkit. Currently 'C-g'.  */
-- (void)userContentController:(WKUserContentController *)userContentController
+- (void) userContentController:(WKUserContentController *)userContentController
       didReceiveScriptMessage:(WKScriptMessage *)message
 {
   if ([message.body isEqualToString:@"C-g"])
@@ -343,6 +370,20 @@ - (void)userContentController:(WKUserContentController *)userContentController
   }
 }
 
+double
+nsxwidget_webkit_estimated_load_progress (struct xwidget *xw)
+{
+  XwWebView *xwWebView = (XwWebView *) xw->xwWidget;
+  return xwWebView.estimatedProgress;
+}
+
+void
+nsxwidget_webkit_stop_loading (struct xwidget *xw)
+{
+  XwWebView *xwWebView = (XwWebView *) xw->xwWidget;
+  [xwWebView stopLoading];
+}
+
 void
 nsxwidget_webkit_zoom (struct xwidget *xw, double zoom_change)
 {
@@ -430,7 +471,7 @@ - (void)userContentController:(WKUserContentController *)userContentController
         }
       else if (result && FUNCTIONP (fun))
         {
-          // NSLog (@"result=%@, type=%@", result, [result class]);
+          /* NSLog (@"result=%@, type=%@", result, [result class]); */
           Lisp_Object lisp_value = js_to_lisp (result);
           store_xwidget_js_callback_event (xw, fun, lisp_value);
         }
@@ -440,19 +481,20 @@ - (void)userContentController:(WKUserContentController *)userContentController
 /* Window containing an xwidget.  */
 
 @implementation XwWindow
-- (BOOL)isFlipped { return YES; }
+- (BOOL) isFlipped { return YES; }
 @end
 
 /* Xwidget model, macOS Cocoa part.  */
 
 void
-nsxwidget_init(struct xwidget *xw)
+nsxwidget_init (struct xwidget *xw)
 {
   block_input ();
   NSRect rect = NSMakeRect (0, 0, xw->width, xw->height);
   xw->xwWidget = [[XwWebView alloc]
                    initWithFrame:rect
-                   configuration:[[WKWebViewConfiguration alloc] init]
+                   configuration:[[[WKWebViewConfiguration alloc] init]
+                                   autorelease]
                          xwidget:xw];
   xw->xwWindow = [[XwWindow alloc]
                    initWithFrame:rect];
@@ -470,16 +512,18 @@ - (BOOL)isFlipped { return YES; }
         ((XwWebView *) xw->xwWidget).configuration.userContentController;
       [scriptor removeAllUserScripts];
       [scriptor removeScriptMessageHandlerForName:@"keyDown"];
-      [scriptor release];
+
       if (xw->xv)
         xw->xv->model = Qnil; /* Make sure related view stale.  */
 
       /* This stops playing audio when a xwidget-webkit buffer is
-         killed.  I could not find other solution.  */
+         killed.  I could not find other solution.
+         TODO: improve this */
       nsxwidget_webkit_goto_uri (xw, "about:blank");
 
       [((XwWebView *) xw->xwWidget).urlScriptBlocked release];
       [xw->xwWidget removeFromSuperviewWithoutNeedingDisplay];
+
       [xw->xwWidget release];
       [xw->xwWindow removeFromSuperviewWithoutNeedingDisplay];
       [xw->xwWindow release];
@@ -507,7 +551,7 @@ - (BOOL)isFlipped { return YES; }
 /* Xwidget view, macOS Cocoa part.  */
 
 @implementation XvWindow : NSView
-- (BOOL)isFlipped { return YES; }
+- (BOOL) isFlipped { return YES; }
 @end
 
 void
diff --git a/src/xwidget.c b/src/xwidget.c
index efe2705562..7f30e48c95 100644
--- a/src/xwidget.c
+++ b/src/xwidget.c
@@ -3063,6 +3063,36 @@ DEFUN ("xwidget-webkit-title",
 #endif
 }
 
+DEFUN ("xwidget-webkit-estimated-load-progress",
+       Fxwidget_webkit_estimated_load_progress, Sxwidget_webkit_estimated_load_progress,
+       1, 1, 0, doc: /* Get the estimated load progress of XWIDGET, a WebKit widget.
+Return a value ranging from 0.0 to 1.0, based on how close XWIDGET
+is to completely loading its page.  */)
+  (Lisp_Object xwidget)
+{
+  struct xwidget *xw;
+#ifdef USE_GTK
+  WebKitWebView *webview;
+#endif
+  double value;
+
+  CHECK_LIVE_XWIDGET (xwidget);
+  xw = XXWIDGET (xwidget);
+  CHECK_WEBKIT_WIDGET (xw);
+
+  block_input ();
+#ifdef USE_GTK
+  webview = WEBKIT_WEB_VIEW (xw->widget_osr);
+  value = webkit_web_view_get_estimated_load_progress (webview);
+#elif defined NS_IMPL_COCOA
+  value = nsxwidget_webkit_estimated_load_progress (xw);
+#endif
+
+  unblock_input ();
+
+  return make_float (value);
+}
+
 DEFUN ("xwidget-webkit-goto-uri",
        Fxwidget_webkit_goto_uri, Sxwidget_webkit_goto_uri,
        2, 2, 0,
@@ -3810,28 +3840,6 @@ DEFUN ("xwidget-webkit-back-forward-list", Fxwidget_webkit_back_forward_list,
   return list3 (back, here, forward);
 }
 
-DEFUN ("xwidget-webkit-estimated-load-progress",
-       Fxwidget_webkit_estimated_load_progress, Sxwidget_webkit_estimated_load_progress,
-       1, 1, 0, doc: /* Get the estimated load progress of XWIDGET, a WebKit widget.
-Return a value ranging from 0.0 to 1.0, based on how close XWIDGET
-is to completely loading its page.  */)
-  (Lisp_Object xwidget)
-{
-  struct xwidget *xw;
-  WebKitWebView *webview;
-  double value;
-
-  CHECK_LIVE_XWIDGET (xwidget);
-  xw = XXWIDGET (xwidget);
-  CHECK_WEBKIT_WIDGET (xw);
-
-  block_input ();
-  webview = WEBKIT_WEB_VIEW (xw->widget_osr);
-  value = webkit_web_view_get_estimated_load_progress (webview);
-  unblock_input ();
-
-  return make_float (value);
-}
 #endif
 
 DEFUN ("xwidget-webkit-set-cookie-storage-file",
@@ -3874,19 +3882,23 @@ DEFUN ("xwidget-webkit-stop-loading", Fxwidget_webkit_stop_loading,
 XWIDGET as part of loading a page.  */)
   (Lisp_Object xwidget)
 {
-#ifdef USE_GTK
   struct xwidget *xw;
+#ifdef USE_GTK
   WebKitWebView *webview;
+#endif
 
   CHECK_LIVE_XWIDGET (xwidget);
   xw = XXWIDGET (xwidget);
   CHECK_WEBKIT_WIDGET (xw);
 
   block_input ();
+#ifdef USE_GTK
   webview = WEBKIT_WEB_VIEW (xw->widget_osr);
   webkit_web_view_stop_loading (webview);
-  unblock_input ();
+#elif defined NS_IMPL_COCOA
+  nsxwidget_webkit_stop_loading (xw);
 #endif
+  unblock_input ();
 
   return Qnil;
 }
@@ -3936,8 +3948,9 @@ syms_of_xwidget (void)
 #ifdef USE_GTK
   defsubr (&Sxwidget_webkit_load_html);
   defsubr (&Sxwidget_webkit_back_forward_list);
-  defsubr (&Sxwidget_webkit_estimated_load_progress);
 #endif
+
+  defsubr (&Sxwidget_webkit_estimated_load_progress);
   defsubr (&Skill_xwidget);
 
   DEFSYM (QCxwidget, ":xwidget");

bug#60703: Patches to xwidget code

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Andrew De Angelis <bobodeangelis@gmail.com> writes:

> Thanks for the explanation. I sent out the signed copy yesterday. 
> I noticed a couple things about the previous patch, so I'm sending this one with a few updates:
>
> * `xwidget-webkit-current-url` in lisp/xwidget.el: check if the url variable is non-nil before calling `kill-new' on it. This avoids killing an empty
>  string, which would be pointless. We still alert the user that something's wrong by messaging "URL: nil" (although getting a nil url seems
>  very unlikely).
> * in src/nsxwidget.m: formatting (keep lines below 80 char), and a brief comment describing the purpose of some of the newly added
>  functions
>
> I'm also attaching a draft of the ChangeLog. Let me know if you'd like me to make any changes there. 
>
> I do have a question about the X11/GTK implementations for xwidget. I'm not sure I understand the relationship between the two: from the
> preprocessor macros it seems at times they are separate and at times that one is an addendum to the other.
> I'm asking because I want to make sure I'm using the preprocessor macros correctly: specifically, the function
> `xwidget-webkit-estimated-load-progress` within xwidget.c used to be inside a "#ifdef USE_GTK" block. I moved it outside of the block, and
> separated the GTK implementation from the NS_IMPL_COCOA implementation. Is this a problem for the X11 build? Should I instead put the
> whole function within a block along these lines: "#if defined USE_GTK || defined NS_IMPL_COCOA" ?

No, that's not a problem.  The X11 implementation shares most of the
widget manipulation code with the PGTK one, so both are simply under
``#ifdef USE_GTK''.

bug#60703: Patches to xwidget code

[Andrew De Angelis]

[-- Attachment #1: Type: text/plain, Size: 2085 bytes --]

Hi all, just wanted to check on this: will you be able to include my
changes in Emacs-29?
Don't mean to put pressure or anything, just wondering if there's any
additional things you need from me, and if there's a timeline on when the
changes will be in.

Thanks!

On Thu, Jan 26, 2023 at 7:44 PM Po Lu <luangruo@yahoo.com> wrote:

> Andrew De Angelis <bobodeangelis@gmail.com> writes:
>
> > Thanks for the explanation. I sent out the signed copy yesterday.
> > I noticed a couple things about the previous patch, so I'm sending this
> one with a few updates:
> >
> > * `xwidget-webkit-current-url` in lisp/xwidget.el: check if the url
> variable is non-nil before calling `kill-new' on it. This avoids killing an
> empty
> >  string, which would be pointless. We still alert the user that
> something's wrong by messaging "URL: nil" (although getting a nil url seems
> >  very unlikely).
> > * in src/nsxwidget.m: formatting (keep lines below 80 char), and a brief
> comment describing the purpose of some of the newly added
> >  functions
> >
> > I'm also attaching a draft of the ChangeLog. Let me know if you'd like
> me to make any changes there.
> >
> > I do have a question about the X11/GTK implementations for xwidget. I'm
> not sure I understand the relationship between the two: from the
> > preprocessor macros it seems at times they are separate and at times
> that one is an addendum to the other.
> > I'm asking because I want to make sure I'm using the preprocessor macros
> correctly: specifically, the function
> > `xwidget-webkit-estimated-load-progress` within xwidget.c used to be
> inside a "#ifdef USE_GTK" block. I moved it outside of the block, and
> > separated the GTK implementation from the NS_IMPL_COCOA implementation.
> Is this a problem for the X11 build? Should I instead put the
> > whole function within a block along these lines: "#if defined USE_GTK ||
> defined NS_IMPL_COCOA" ?
>
> No, that's not a problem.  The X11 implementation shares most of the
> widget manipulation code with the PGTK one, so both are simply under
> ``#ifdef USE_GTK''.
>

[-- Attachment #2: Type: text/html, Size: 2667 bytes --]

bug#60703: Patches to xwidget code

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Andrew De Angelis <bobodeangelis@gmail.com> writes:

> Hi all, just wanted to check on this: will you be able to include my changes in Emacs-29?
> Don't mean to put pressure or anything, just wondering if there's any additional things you need from me, and if there's a timeline on when
> the changes will be in.

I think this is up to Eli, but as I said earlier, your changes LGTM.

bug#60703: Patches to xwidget code

[Eli Zaretskii]

> From: Andrew De Angelis <bobodeangelis@gmail.com>
> Date: Sun, 19 Feb 2023 16:00:44 -0500
> Cc: Eli Zaretskii <eliz@gnu.org>, 60703@debbugs.gnu.org
> 
> Hi all, just wanted to check on this: will you be able to include my changes in Emacs-29?
> Don't mean to put pressure or anything, just wondering if there's any additional things you need from me,
> and if there's a timeline on when the changes will be in.

Please post the patches rebased on the current emacs-29 branch.

bug#60703: Patches to xwidget code

[Andrew De Angelis]

[-- Attachment #1.1: Type: text/plain, Size: 704 bytes --]

Done. (git diff forked/emacs-29 > emacs-29-xwidget-patches.patch)

Let me know if there's anything I should do. Thanks!

Best,
Andrew

On Mon, Feb 20, 2023 at 6:47 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Andrew De Angelis <bobodeangelis@gmail.com>
> > Date: Sun, 19 Feb 2023 16:00:44 -0500
> > Cc: Eli Zaretskii <eliz@gnu.org>, 60703@debbugs.gnu.org
> >
> > Hi all, just wanted to check on this: will you be able to include my
> changes in Emacs-29?
> > Don't mean to put pressure or anything, just wondering if there's any
> additional things you need from me,
> > and if there's a timeline on when the changes will be in.
>
> Please post the patches rebased on the current emacs-29 branch.
>

[-- Attachment #1.2: Type: text/html, Size: 1294 bytes --]

[-- Attachment #2: emacs-29-xwidget-patches.patch --]
[-- Type: application/octet-stream, Size: 13815 bytes --]

diff --git a/lisp/xwidget.el b/lisp/xwidget.el
index abbda29081e..7daca81f9f7 100644
--- a/lisp/xwidget.el
+++ b/lisp/xwidget.el
@@ -925,7 +925,8 @@ xwidget-webkit-current-url
   "Display the current xwidget webkit URL and place it on the `kill-ring'."
   (interactive nil xwidget-webkit-mode)
   (let ((url (xwidget-webkit-uri (xwidget-webkit-current-session))))
-    (message "URL: %s" (kill-new (or url "")))))
+    (when url (kill-new url))
+    (message "URL: %s" url)))
 
 (defun xwidget-webkit-browse-history ()
   "Display a buffer containing the history of page loads."
diff --git a/src/nsxwidget.h b/src/nsxwidget.h
index 8d55fac5326..2b5596f905e 100644
--- a/src/nsxwidget.h
+++ b/src/nsxwidget.h
@@ -36,6 +36,8 @@ #define NSXWIDGET_H_INCLUDED
 Lisp_Object nsxwidget_webkit_title (struct xwidget *xw);
 void nsxwidget_webkit_goto_uri (struct xwidget *xw, const char *uri);
 void nsxwidget_webkit_goto_history (struct xwidget *xw, int rel_pos);
+double nsxwidget_webkit_estimated_load_progress(struct xwidget *xw);
+void nsxwidget_webkit_stop_loading (struct xwidget *xw);
 void nsxwidget_webkit_zoom (struct xwidget *xw, double zoom_change);
 void nsxwidget_webkit_execute_script (struct xwidget *xw, const char *script,
                                       Lisp_Object fun);
diff --git a/src/nsxwidget.m b/src/nsxwidget.m
index e1fbd749b62..0e00589bb7f 100644
--- a/src/nsxwidget.m
+++ b/src/nsxwidget.m
@@ -57,12 +57,13 @@ @interface XwWebView : WKWebView
 @end
 @implementation XwWebView : WKWebView
 
-- (id)initWithFrame:(CGRect)frame
+- (id) initWithFrame:(CGRect)frame
       configuration:(WKWebViewConfiguration *)configuration
             xwidget:(struct xwidget *)xw
 {
   /* Script controller to add script message handler and user script.  */
-  WKUserContentController *scriptor = [[WKUserContentController alloc] init];
+  WKUserContentController *scriptor = [[[WKUserContentController alloc] init]
+                                        autorelease];
   configuration.userContentController = scriptor;
 
   /* Enable inspect element context menu item for debugging.  */
@@ -81,7 +82,8 @@ - (id)initWithFrame:(CGRect)frame
   if (self)
     {
       self.xw = xw;
-      self.urlScriptBlocked = [[NSMutableDictionary alloc] init];
+      self.urlScriptBlocked = [[[NSMutableDictionary alloc] init]
+                                autorelease];
       self.navigationDelegate = self;
       self.UIDelegate = self;
       self.customUserAgent =
@@ -89,23 +91,48 @@ - (id)initWithFrame:(CGRect)frame
         @" AppleWebKit/603.3.8 (KHTML, like Gecko)"
         @" Version/11.0.1 Safari/603.3.8";
       [scriptor addScriptMessageHandler:self name:@"keyDown"];
-      [scriptor addUserScript:[[WKUserScript alloc]
-                                initWithSource:xwScript
-                                 injectionTime:
-                                  WKUserScriptInjectionTimeAtDocumentStart
-                                forMainFrameOnly:NO]];
+      WKUserScript *userScript = [[[WKUserScript alloc]
+                                    initWithSource:xwScript
+                                     injectionTime:
+                                      WKUserScriptInjectionTimeAtDocumentStart
+                                    forMainFrameOnly:NO] autorelease];
+      [scriptor addUserScript:userScript];
     }
   return self;
 }
 
-- (void)webView:(WKWebView *)webView
+/* These 4 functions emulate the behavior of webkit_view_load_changed_cb
+   in the GTK implementation*/
+- (void) webView:(WKWebView *)webView
 didFinishNavigation:(WKNavigation *)navigation
 {
   if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
-    store_xwidget_event_string (self.xw, "load-changed", "");
+    store_xwidget_event_string (self.xw, "load-changed", "load-finished");
 }
 
-- (void)webView:(WKWebView *)webView
+- (void) webView:(WKWebView *)webView
+didStartProvisionalNavigation:(WKNavigation *)navigation
+{
+  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
+    store_xwidget_event_string (self.xw, "load-changed", "load-started");
+}
+
+- (void) webView:(WKWebView *)webView
+didReceiveServerRedirectForProvisionalNavigation:(WKNavigation *)navigation
+{
+  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
+    store_xwidget_event_string (self.xw, "load-changed", "load-redirected");
+}
+
+/* Start loading WKWebView */
+- (void) webView:(WKWebView *)webView
+didCommitNavigation:(WKNavigation *)navigation
+{
+  if (EQ (Fbuffer_live_p (self.xw->buffer), Qt))
+    store_xwidget_event_string (self.xw, "load-changed", "load-committed");
+}
+
+- (void) webView:(WKWebView *)webView
 decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction
 decisionHandler:(void (^)(WKNavigationActionPolicy))decisionHandler
 {
@@ -114,13 +141,13 @@ - (void)webView:(WKWebView *)webView
     decisionHandler (WKNavigationActionPolicyAllow);
     break;
   default:
-    // decisionHandler (WKNavigationActionPolicyCancel);
+    /* decisionHandler (WKNavigationActionPolicyCancel); */
     decisionHandler (WKNavigationActionPolicyAllow);
     break;
   }
 }
 
-- (void)webView:(WKWebView *)webView
+- (void) webView:(WKWebView *)webView
 decidePolicyForNavigationResponse:(WKNavigationResponse *)navigationResponse
 decisionHandler:(void (^)(WKNavigationResponsePolicy))decisionHandler
 {
@@ -166,7 +193,7 @@ - (void)webView:(WKWebView *)webView
 
 /* No additional new webview or emacs window will be created
    for <a ... target="_blank">.  */
-- (WKWebView *)webView:(WKWebView *)webView
+- (WKWebView *) webView:(WKWebView *)webView
 createWebViewWithConfiguration:(WKWebViewConfiguration *)configuration
    forNavigationAction:(WKNavigationAction *)navigationAction
         windowFeatures:(WKWindowFeatures *)windowFeatures
@@ -177,7 +204,7 @@ - (WKWebView *)webView:(WKWebView *)webView
 }
 
 /* Open panel for file upload.  */
-- (void)webView:(WKWebView *)webView
+- (void) webView:(WKWebView *)webView
 runOpenPanelWithParameters:(WKOpenPanelParameters *)parameters
 initiatedByFrame:(WKFrameInfo *)frame
 completionHandler:(void (^)(NSArray<NSURL *> *URLs))completionHandler
@@ -197,13 +224,13 @@ - (void)webView:(WKWebView *)webView
    - Correct mouse hand/arrow/I-beam is displayed (TODO: not perfect yet).
 */
 
-- (void)mouseDown:(NSEvent *)event
+- (void) mouseDown:(NSEvent *)event
 {
   [self.xw->xv->emacswindow mouseDown:event];
   [super mouseDown:event];
 }
 
-- (void)mouseUp:(NSEvent *)event
+- (void) mouseUp:(NSEvent *)event
 {
   [self.xw->xv->emacswindow mouseUp:event];
   [super mouseUp:event];
@@ -214,7 +241,7 @@ - (void)mouseUp:(NSEvent *)event
    emacs as first responder to avoid focus held in an input element
    with matching text.  */
 
-- (void)keyDown:(NSEvent *)event
+- (void) keyDown:(NSEvent *)event
 {
   Lisp_Object var = Fintern (build_string ("isearch-mode"), Qnil);
   Lisp_Object val = buffer_local_value (var, Fcurrent_buffer ());
@@ -250,7 +277,7 @@ - (void)keyDown:(NSEvent *)event
     }];
 }
 
-- (void)interpretKeyEvents:(NSArray<NSEvent *> *)eventArray
+- (void) interpretKeyEvents:(NSArray<NSEvent *> *)eventArray
 {
   /* We should do nothing and do not forward (default implementation
      if we not override here) to let emacs collect key events and ask
@@ -258,7 +285,7 @@ - (void)interpretKeyEvents:(NSArray<NSEvent *> *)eventArray
 }
 
 static NSString *xwScript;
-+ (void)initialize
++ (void) initialize
 {
   /* Find out if an input element has focus.
      Message to script message handler when 'C-g' key down.  */
@@ -284,7 +311,7 @@ + (void)initialize
 
 /* Confirming to WKScriptMessageHandler, listens concerning keyDown in
    webkit. Currently 'C-g'.  */
-- (void)userContentController:(WKUserContentController *)userContentController
+- (void) userContentController:(WKUserContentController *)userContentController
       didReceiveScriptMessage:(WKScriptMessage *)message
 {
   if ([message.body isEqualToString:@"C-g"])
@@ -343,6 +370,20 @@ - (void)userContentController:(WKUserContentController *)userContentController
   }
 }
 
+double
+nsxwidget_webkit_estimated_load_progress (struct xwidget *xw)
+{
+  XwWebView *xwWebView = (XwWebView *) xw->xwWidget;
+  return xwWebView.estimatedProgress;
+}
+
+void
+nsxwidget_webkit_stop_loading (struct xwidget *xw)
+{
+  XwWebView *xwWebView = (XwWebView *) xw->xwWidget;
+  [xwWebView stopLoading];
+}
+
 void
 nsxwidget_webkit_zoom (struct xwidget *xw, double zoom_change)
 {
@@ -430,7 +471,7 @@ - (void)userContentController:(WKUserContentController *)userContentController
         }
       else if (result && FUNCTIONP (fun))
         {
-          // NSLog (@"result=%@, type=%@", result, [result class]);
+          /* NSLog (@"result=%@, type=%@", result, [result class]); */
           Lisp_Object lisp_value = js_to_lisp (result);
           store_xwidget_js_callback_event (xw, fun, lisp_value);
         }
@@ -440,19 +481,20 @@ - (void)userContentController:(WKUserContentController *)userContentController
 /* Window containing an xwidget.  */
 
 @implementation XwWindow
-- (BOOL)isFlipped { return YES; }
+- (BOOL) isFlipped { return YES; }
 @end
 
 /* Xwidget model, macOS Cocoa part.  */
 
 void
-nsxwidget_init(struct xwidget *xw)
+nsxwidget_init (struct xwidget *xw)
 {
   block_input ();
   NSRect rect = NSMakeRect (0, 0, xw->width, xw->height);
   xw->xwWidget = [[XwWebView alloc]
                    initWithFrame:rect
-                   configuration:[[WKWebViewConfiguration alloc] init]
+                   configuration:[[[WKWebViewConfiguration alloc] init]
+                                   autorelease]
                          xwidget:xw];
   xw->xwWindow = [[XwWindow alloc]
                    initWithFrame:rect];
@@ -470,16 +512,18 @@ - (BOOL)isFlipped { return YES; }
         ((XwWebView *) xw->xwWidget).configuration.userContentController;
       [scriptor removeAllUserScripts];
       [scriptor removeScriptMessageHandlerForName:@"keyDown"];
-      [scriptor release];
+
       if (xw->xv)
         xw->xv->model = Qnil; /* Make sure related view stale.  */
 
       /* This stops playing audio when a xwidget-webkit buffer is
-         killed.  I could not find other solution.  */
+         killed.  I could not find other solution.
+         TODO: improve this */
       nsxwidget_webkit_goto_uri (xw, "about:blank");
 
       [((XwWebView *) xw->xwWidget).urlScriptBlocked release];
       [xw->xwWidget removeFromSuperviewWithoutNeedingDisplay];
+
       [xw->xwWidget release];
       [xw->xwWindow removeFromSuperviewWithoutNeedingDisplay];
       [xw->xwWindow release];
@@ -507,7 +551,7 @@ - (BOOL)isFlipped { return YES; }
 /* Xwidget view, macOS Cocoa part.  */
 
 @implementation XvWindow : NSView
-- (BOOL)isFlipped { return YES; }
+- (BOOL) isFlipped { return YES; }
 @end
 
 void
diff --git a/src/xwidget.c b/src/xwidget.c
index efe27055629..7f30e48c954 100644
--- a/src/xwidget.c
+++ b/src/xwidget.c
@@ -3063,6 +3063,36 @@ DEFUN ("xwidget-webkit-title",
 #endif
 }
 
+DEFUN ("xwidget-webkit-estimated-load-progress",
+       Fxwidget_webkit_estimated_load_progress, Sxwidget_webkit_estimated_load_progress,
+       1, 1, 0, doc: /* Get the estimated load progress of XWIDGET, a WebKit widget.
+Return a value ranging from 0.0 to 1.0, based on how close XWIDGET
+is to completely loading its page.  */)
+  (Lisp_Object xwidget)
+{
+  struct xwidget *xw;
+#ifdef USE_GTK
+  WebKitWebView *webview;
+#endif
+  double value;
+
+  CHECK_LIVE_XWIDGET (xwidget);
+  xw = XXWIDGET (xwidget);
+  CHECK_WEBKIT_WIDGET (xw);
+
+  block_input ();
+#ifdef USE_GTK
+  webview = WEBKIT_WEB_VIEW (xw->widget_osr);
+  value = webkit_web_view_get_estimated_load_progress (webview);
+#elif defined NS_IMPL_COCOA
+  value = nsxwidget_webkit_estimated_load_progress (xw);
+#endif
+
+  unblock_input ();
+
+  return make_float (value);
+}
+
 DEFUN ("xwidget-webkit-goto-uri",
        Fxwidget_webkit_goto_uri, Sxwidget_webkit_goto_uri,
        2, 2, 0,
@@ -3810,28 +3840,6 @@ DEFUN ("xwidget-webkit-back-forward-list", Fxwidget_webkit_back_forward_list,
   return list3 (back, here, forward);
 }
 
-DEFUN ("xwidget-webkit-estimated-load-progress",
-       Fxwidget_webkit_estimated_load_progress, Sxwidget_webkit_estimated_load_progress,
-       1, 1, 0, doc: /* Get the estimated load progress of XWIDGET, a WebKit widget.
-Return a value ranging from 0.0 to 1.0, based on how close XWIDGET
-is to completely loading its page.  */)
-  (Lisp_Object xwidget)
-{
-  struct xwidget *xw;
-  WebKitWebView *webview;
-  double value;
-
-  CHECK_LIVE_XWIDGET (xwidget);
-  xw = XXWIDGET (xwidget);
-  CHECK_WEBKIT_WIDGET (xw);
-
-  block_input ();
-  webview = WEBKIT_WEB_VIEW (xw->widget_osr);
-  value = webkit_web_view_get_estimated_load_progress (webview);
-  unblock_input ();
-
-  return make_float (value);
-}
 #endif
 
 DEFUN ("xwidget-webkit-set-cookie-storage-file",
@@ -3874,19 +3882,23 @@ DEFUN ("xwidget-webkit-stop-loading", Fxwidget_webkit_stop_loading,
 XWIDGET as part of loading a page.  */)
   (Lisp_Object xwidget)
 {
-#ifdef USE_GTK
   struct xwidget *xw;
+#ifdef USE_GTK
   WebKitWebView *webview;
+#endif
 
   CHECK_LIVE_XWIDGET (xwidget);
   xw = XXWIDGET (xwidget);
   CHECK_WEBKIT_WIDGET (xw);
 
   block_input ();
+#ifdef USE_GTK
   webview = WEBKIT_WEB_VIEW (xw->widget_osr);
   webkit_web_view_stop_loading (webview);
-  unblock_input ();
+#elif defined NS_IMPL_COCOA
+  nsxwidget_webkit_stop_loading (xw);
 #endif
+  unblock_input ();
 
   return Qnil;
 }
@@ -3936,8 +3948,9 @@ syms_of_xwidget (void)
 #ifdef USE_GTK
   defsubr (&Sxwidget_webkit_load_html);
   defsubr (&Sxwidget_webkit_back_forward_list);
-  defsubr (&Sxwidget_webkit_estimated_load_progress);
 #endif
+
+  defsubr (&Sxwidget_webkit_estimated_load_progress);
   defsubr (&Skill_xwidget);
 
   DEFSYM (QCxwidget, ":xwidget");

bug#60703: Patches to xwidget code

[Eli Zaretskii]

> From: Andrew De Angelis <bobodeangelis@gmail.com>
> Date: Tue, 21 Feb 2023 00:24:06 -0500
> Cc: luangruo@yahoo.com, 60703@debbugs.gnu.org
> 
> Done. (git diff forked/emacs-29 > emacs-29-xwidget-patches.patch)
> 
> Let me know if there's anything I should do. Thanks!

Thanks, what is missing now is the commit log message.  Please see
CONTRIBUTE and the examples in the Git repository, for how we format
the log messages.

One minor comment to the code:

> +DEFUN ("xwidget-webkit-estimated-load-progress",
> +       Fxwidget_webkit_estimated_load_progress, Sxwidget_webkit_estimated_load_progress,
> +       1, 1, 0, doc: /* Get the estimated load progress of XWIDGET, a WebKit widget.
> +Return a value ranging from 0.0 to 1.0, based on how close XWIDGET
> +is to completely loading its page.  */)

The first line of a doc string should be a single complete sentence.
This is because commands like "M-x apropos" show only the first line.

(Yes, I'm aware that you just moved existing code, but still: let's
fix this while we are at that.)

bug#60703: Patches to xwidget code

[Andrew De Angelis]

[-- Attachment #1.1: Type: text/plain, Size: 1574 bytes --]

Let me know if this ChangeLog is fine or you'd like me to make any changes.

The first line of a doc string should be a single complete sentence.
>
I'm not sure I follow: in this case, isn't  this the first line: "Get the
estimated load progress of XWIDGET, a WebKit widget."? It renders fine when
I run `M-x apropos xwidget-webkit-estimated-load-progress`. But happy to
make any additional changes if needed.



On Wed, Feb 22, 2023 at 8:26 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Andrew De Angelis <bobodeangelis@gmail.com>
> > Date: Tue, 21 Feb 2023 00:24:06 -0500
> > Cc: luangruo@yahoo.com, 60703@debbugs.gnu.org
> >
> > Done. (git diff forked/emacs-29 > emacs-29-xwidget-patches.patch)
> >
> > Let me know if there's anything I should do. Thanks!
>
> Thanks, what is missing now is the commit log message.  Please see
> CONTRIBUTE and the examples in the Git repository, for how we format
> the log messages.
>
> One minor comment to the code:
>
> > +DEFUN ("xwidget-webkit-estimated-load-progress",
> > +       Fxwidget_webkit_estimated_load_progress,
> Sxwidget_webkit_estimated_load_progress,
> > +       1, 1, 0, doc: /* Get the estimated load progress of XWIDGET, a
> WebKit widget.
> > +Return a value ranging from 0.0 to 1.0, based on how close XWIDGET
> > +is to completely loading its page.  */)
>
> The first line of a doc string should be a single complete sentence.
> This is because commands like "M-x apropos" show only the first line.
>
> (Yes, I'm aware that you just moved existing code, but still: let's
> fix this while we are at that.)
>

[-- Attachment #1.2: Type: text/html, Size: 2447 bytes --]

[-- Attachment #2: ChangeLog.andrewda --]
[-- Type: application/octet-stream, Size: 1320 bytes --]

2023-02-23  Andrew De Angelis  <bobodeangelis@gmail.com>
	Improvements to xwidget

	* src/nsxwidget.m ()
	([XwWebView initWithFrame:configuration:xwidget:])
	(nsxwidget_init):  Fixed memory leaks: when sending an alloc message to
	an object, send an autorelease message to any objects we won't
	explictly release
	([XwWebView webView:didFinishNavigation:]): Second string to store in
	'store_xwidget_event_string' is "load finished" rather than empty string
	([XwWebView webView:didStartProvisionalNavigation:])
	([XwWebView webView:didReceiveServerRedirectForProvisionalNavigation:])
	([XwWebView webView:didCommitNavigation:]): New functions
	(nsxwidget_webkit_estimated_load_progress): New function
	(nsxwidget_webkit_stop_loading): New function
	* src/xwidget.c (Fxwidget_webkit_estimated_load_progress): Call
	'nsxwidget_webkit_estimated_load_progress' if we're on MacOS
	(Fxwidget_webkit_stop_loading): Call 'nsxwidget_webkit_stop_loading' if
	we're on MacOS
	(syms_of_xwidget): Define symbol for function
	'xwidget_webkit_estimated_load_progress' if we're on MacOS
	* src/nsxwidget.h: signature for functions
	'nsxwidget_webkit_estimated_load_progress' and
	'nsxwidget_webkit_stop_loading'
	* lisp/xwidget.el (xwidget-webkit-current-url): Message URL rather than
	return value of 'kill-new' (which is always nil)

bug#60703: Patches to xwidget code

[Eli Zaretskii]

> From: Andrew De Angelis <bobodeangelis@gmail.com>
> Date: Thu, 23 Feb 2023 22:47:41 -0500
> Cc: luangruo@yahoo.com, 60703@debbugs.gnu.org
> 
> Let me know if this ChangeLog is fine or you'd like me to make any changes.

Thanks, installed on the emacs-29 branch, and closing the bug.

>  The first line of a doc string should be a single complete sentence.
> 
> I'm not sure I follow: in this case, isn't  this the first line: "Get the estimated load progress of XWIDGET, a
> WebKit widget."? It renders fine when I run `M-x apropos xwidget-webkit-estimated-load-progress`. But
> happy to make any additional changes if needed.

Please ignore that part: I was confused.

Please in the future always accompany the changes with a
ChangeLog-style commit log message, and mention the bug number there
if you know it.  Also, use of "git format-patch" is encouraged.  These
measures make the job of installing your changes much easier.

bug#60703: Patches to xwidget code

[Andrew De Angelis]

[-- Attachment #1: Type: text/plain, Size: 1305 bytes --]

Awesome, thank you so much! Will keep all your suggestions in mind for my
future contributions.

Small question: what are the criteria to be added to `etc/AUTHORS`? Just
wondering if/how my contribution could be mentioned there.

On Thu, Mar 2, 2023 at 5:58 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Andrew De Angelis <bobodeangelis@gmail.com>
> > Date: Thu, 23 Feb 2023 22:47:41 -0500
> > Cc: luangruo@yahoo.com, 60703@debbugs.gnu.org
> >
> > Let me know if this ChangeLog is fine or you'd like me to make any
> changes.
>
> Thanks, installed on the emacs-29 branch, and closing the bug.
>
> >  The first line of a doc string should be a single complete sentence.
> >
> > I'm not sure I follow: in this case, isn't  this the first line: "Get
> the estimated load progress of XWIDGET, a
> > WebKit widget."? It renders fine when I run `M-x apropos
> xwidget-webkit-estimated-load-progress`. But
> > happy to make any additional changes if needed.
>
> Please ignore that part: I was confused.
>
> Please in the future always accompany the changes with a
> ChangeLog-style commit log message, and mention the bug number there
> if you know it.  Also, use of "git format-patch" is encouraged.  These
> measures make the job of installing your changes much easier.
>

[-- Attachment #2: Type: text/html, Size: 1890 bytes --]

bug#60703: Patches to xwidget code

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Andrew De Angelis <bobodeangelis@gmail.com> writes:

> Awesome, thank you so much! Will keep all your suggestions in mind for my future contributions.
>
> Small question: what are the criteria to be added to `etc/AUTHORS`? Just wondering if/how my contribution could be mentioned there.

AUTHORS is generated automatically prior to each release of Emacs, so
you don't have to do anything aside from waiting for Emacs 29.1 to be
released.

Thanks.