bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

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

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

Thanks!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-hang-when-displaying-xwidget-script-dialog.patch --]
[-- Type: text/x-patch, Size: 3374 bytes --]

From 1ce79e6631678dd0c98718bbb0882cc6c169d2b0 Mon Sep 17 00:00:00 2001
From: Po Lu <luangruo@yahoo.com>
Date: Mon, 8 Nov 2021 10:07:55 +0800
Subject: [PATCH] Fix hang when displaying xwidget script dialog

* src/xwidget.c (webkit_script_dialog_cb): New function.
(Fmake_xwidget): Attach script callback signal.
---
 src/xwidget.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/src/xwidget.c b/src/xwidget.c
index f95f5f1d7f..ecb973f485 100644
--- a/src/xwidget.c
+++ b/src/xwidget.c
@@ -50,6 +50,8 @@ Copyright (C) 2011-2021 Free Software Foundation, Inc.
 static gboolean offscreen_damage_event (GtkWidget *, GdkEvent *, gpointer);
 static void synthesize_focus_in_event (GtkWidget *);
 static GdkDevice *find_suitable_keyboard (struct frame *);
+static gboolean webkit_script_dialog_cb (WebKitWebView *, WebKitScriptDialog *,
+					 gpointer);
 #endif
 
 static struct xwidget *
@@ -240,6 +242,10 @@ DEFUN ("make-xwidget",
 			    "create",
 			    G_CALLBACK (webkit_create_cb),
 			    xw);
+	  g_signal_connect (G_OBJECT (xw->widget_osr),
+			    "script-dialog",
+			    G_CALLBACK (webkit_script_dialog_cb),
+			    NULL);
         }
 
       g_signal_connect (G_OBJECT (xw->widgetwindow_osr), "damage-event",
@@ -1242,6 +1248,66 @@ webkit_decide_policy_cb (WebKitWebView *webView,
     return FALSE;
   }
 }
+
+static gboolean
+webkit_script_dialog_cb (WebKitWebView *webview,
+			 WebKitScriptDialog *script_dialog,
+			 gpointer user)
+{
+  struct frame *f = SELECTED_FRAME ();
+  WebKitScriptDialogType type;
+  GtkWidget *widget;
+  GtkWidget *dialog;
+  GtkWidget *entry;
+  GtkWidget *content_area;
+  const gchar *content;
+  const gchar *message;
+  gint result;
+
+  /* Return TRUE to prevent WebKit from showing the default script
+     dialog in the offscreen window, which runs a nested main loop
+     Emacs can't respond to, and as such can't pass X events to.  */
+  if (!FRAME_WINDOW_P (f))
+    return TRUE;
+
+  type = webkit_script_dialog_get_dialog_type (script_dialog);;
+  widget = FRAME_GTK_OUTER_WIDGET (f);
+  content = webkit_script_dialog_get_message (script_dialog);
+
+  if (type == WEBKIT_SCRIPT_DIALOG_ALERT)
+    dialog = gtk_dialog_new_with_buttons (content, GTK_WINDOW (widget),
+					  GTK_DIALOG_MODAL,
+					  "Dismiss", 1, NULL);
+  else
+    dialog = gtk_dialog_new_with_buttons (content, GTK_WINDOW (widget),
+					  GTK_DIALOG_MODAL,
+					  "OK", 0, "Cancel", 1, NULL);
+
+  if (type == WEBKIT_SCRIPT_DIALOG_PROMPT)
+    {
+      entry = gtk_entry_new ();
+      message = webkit_script_dialog_prompt_get_default_text (script_dialog);
+      content_area = gtk_dialog_get_content_area (GTK_DIALOG (dialog));
+
+      gtk_widget_show (entry);
+      gtk_entry_set_text (GTK_ENTRY (entry), message);
+      gtk_container_add (GTK_CONTAINER (content_area), entry);
+    }
+
+  result = gtk_dialog_run (GTK_DIALOG (dialog));
+
+  if (type == WEBKIT_SCRIPT_DIALOG_CONFIRM
+      || type == WEBKIT_SCRIPT_DIALOG_BEFORE_UNLOAD_CONFIRM)
+    webkit_script_dialog_confirm_set_confirmed (script_dialog, result == 0);
+
+  if (type == WEBKIT_SCRIPT_DIALOG_PROMPT)
+    webkit_script_dialog_prompt_set_text (script_dialog,
+					  gtk_entry_get_text (GTK_ENTRY (entry)));
+
+  gtk_widget_destroy (GTK_WIDGET (dialog));
+
+  return TRUE;
+}
 #endif /* USE_GTK */
 
 
-- 
2.31.1

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

[Lars Ingebrigtsen]

Po Lu <luangruo@yahoo.com> writes:

> Thanks!

Thanks; pushed to Emacs 29.

This reminds me of a totally different thing that I was poking at
briefly some years back -- is it possible to use xwidgets "freely" in a
different buffer these days?  Last time I looked at it, it was very much
tied to a specific mode, and only one widget per buffer.

My use case here is that I'd love to see eww being able to display video
clips.  xwidget can show <video> elements just fine, but it'd be great
to have them directly in the eww buffer (and perhaps several playing at
the same time, etc).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> This reminds me of a totally different thing that I was poking at
> briefly some years back -- is it possible to use xwidgets "freely" in a
> different buffer these days?  Last time I looked at it, it was very much
> tied to a specific mode, and only one widget per buffer.

You can show multiple widgets in one buffer today, which wasn't the
situation a couple of years back.

> My use case here is that I'd love to see eww being able to display video
> clips.  xwidget can show <video> elements just fine, but it'd be great
> to have them directly in the eww buffer (and perhaps several playing at
> the same time, etc).

It should work, but take care that the _same_ xwidget is never displayed
on-screen in more than one place at any given time.  If it is, then one
of the xwidget views will not display.

This is a known problem that I'm trying to resolve, it will probably be
a temporary limitation.  Thanks.

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

[Lars Ingebrigtsen]

Po Lu <luangruo@yahoo.com> writes:

> You can show multiple widgets in one buffer today, which wasn't the
> situation a couple of years back.

Great!  What's the function to insert an xwidget at point in a buffer?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Great!  What's the function to insert an xwidget at point in a buffer?

`xwidget-insert', but I get the feeling it's an API intended to be
private to xwidget-webkit-mode.  I don't know, as I wasn't the author
of most of the user facing xwidget-webkit-mode code.

What I would do instead is to use `make-xwidget' to create an xwidget,
and then to make it the display property of a piece of text, like one
would an image spec.

Thanks.

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

[Lars Ingebrigtsen]

Po Lu <luangruo@yahoo.com> writes:

> What I would do instead is to use `make-xwidget' to create an xwidget,
> and then to make it the display property of a piece of text, like one
> would an image spec.

Thanks; this works, sort of:

(progn
  (require 'xwidget)
  (setq widget (make-xwidget 'webkit
			     "Video"
			     700
			     500
			     nil
			     (current-buffer)
			     (xwidget-webkit-current-session)))
  (insert
   (propertize
    "[video]"
    'display (list 'xwidget :xwidget widget)))
  (xwidget-put widget 'callback #'always))

And then

(xwidget-webkit-goto-uri widget "file:///tmp/vid.html")

will play the .mp4 video...  but only on Macos.  On this Debian laptop,
it just shows the controls, and doesn't play the mp4.  Is this due to
platform specific limitations?  (It won't play Youtube, either, with the
normal `xwidget-webkit-browse-url'.)

But on Macos there's a different twist: It doesn't heed the width/height
specs, and always maximises itself to fill the frame.  Which seems like
a bug.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> (progn
>   (require 'xwidget)
>   (setq widget (make-xwidget 'webkit
> 			     "Video"
> 			     700
> 			     500
> 			     nil
> 			     (current-buffer)
> 			     (xwidget-webkit-current-session)))
>   (insert
>    (propertize
>     "[video]"
>     'display (list 'xwidget :xwidget widget)))
>   (xwidget-put widget 'callback #'always))

> (xwidget-webkit-goto-uri widget "file:///tmp/vid.html")

> will play the .mp4 video...  but only on Macos.  On this Debian laptop,
> it just shows the controls, and doesn't play the mp4.  Is this due to
> platform specific limitations?  (It won't play Youtube, either, with the
> normal `xwidget-webkit-browse-url'.)

That's weird, because video works here.  Does it work in another
WebKitGTK based browser, like Epiphany, on your Debian system?  Thanks.

> But on Macos there's a different twist: It doesn't heed the width/height
> specs, and always maximises itself to fill the frame.  Which seems like
> a bug.

Unfortunately I don't know enough about macOS to solve the problem here.

But try removing this snippet of x_draw_xwidget_glyph_string:


  /* On X11, this keeps generating expose events.  */
#ifndef USE_GTK
  /* Resize xwidget webkit if its container window size is changed in
     some ways, for example, a buffer became hidden in small split
     window, then it can appear front in merged whole window.  */
  if (EQ (xww->type, Qwebkit)
      && (xww->width != text_area_width || xww->height != text_area_height))
    {
      Lisp_Object xwl;
      XSETXWIDGET (xwl, xww);
      Fxwidget_resize (xwl,
                       make_int (text_area_width),
                       make_int (text_area_height));
    }
#endif

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

[Lars Ingebrigtsen]

Po Lu <luangruo@yahoo.com> writes:

> That's weird, because video works here.  Does it work in another
> WebKitGTK based browser, like Epiphany, on your Debian system?  Thanks.

I tried Epiphany now, and it works there.  That is, it plays the mp4
very slowly, but it does play it.  In the xwidget, it just puts up the
controls, and then nothing happens.  (This happens with `M-x
xwidget-webkit-browse-url', too, so it's not just this way of creating
the xwidgets that's stopping the mp4s from working.)

> Unfortunately I don't know enough about macOS to solve the problem here.
>
> But try removing this snippet of x_draw_xwidget_glyph_string:

Yes, indeed, that fixed it, so I now have basic <video> support working
in eww.  😹  But only on Macos.

>   /* On X11, this keeps generating expose events.  */
> #ifndef USE_GTK
>   /* Resize xwidget webkit if its container window size is changed in
>      some ways, for example, a buffer became hidden in small split
>      window, then it can appear front in merged whole window.  */
>   if (EQ (xww->type, Qwebkit)
>       && (xww->width != text_area_width || xww->height != text_area_height))
>     {
>       Lisp_Object xwl;
>       XSETXWIDGET (xwl, xww);
>       Fxwidget_resize (xwl,
>                        make_int (text_area_width),
>                        make_int (text_area_height));
>     }
> #endif

I'm not that familiar with the xwidget.c code...  but that's just always
maximalising the widget to be as big as the Emacs window?  That can't be
correct, surely.  Should I just remove it?  It doesn't seem to lead to
any regressions in xwidget-webkit-browse-url, either.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> That's weird, because video works here.  Does it work in another
>> WebKitGTK based browser, like Epiphany, on your Debian system?  Thanks.

> I tried Epiphany now, and it works there.  That is, it plays the mp4
> very slowly, but it does play it.  In the xwidget, it just puts up the
> controls, and then nothing happens.  (This happens with `M-x
> xwidget-webkit-browse-url', too, so it's not just this way of creating
> the xwidgets that's stopping the mp4s from working.)

Can you test the xwidget support using self-built WebKitGTK with
`-DENABLE_GL=NO' added to your cmake options?

You can follow the instructions here: https://trac.webkit.org/wiki/BuildingGtk.

Afterwards, you can add the path to the library into your
LD_LIBRARY_PATH, and start Emacs that way.

Thanks a lot!

> I'm not that familiar with the xwidget.c code...  but that's just always
> maximalising the widget to be as big as the Emacs window?  That can't be
> correct, surely.  Should I just remove it?  It doesn't seem to lead to
> any regressions in xwidget-webkit-browse-url, either.

Yes, you should remove it.  I suspect that code was originally added to
work around the resizing bug I fixed in bug#51679.  Thanks.

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

[Lars Ingebrigtsen]

Po Lu <luangruo@yahoo.com> writes:

> Can you test the xwidget support using self-built WebKitGTK with
> `-DENABLE_GL=NO' added to your cmake options?
>
> You can follow the instructions here: https://trac.webkit.org/wiki/BuildingGtk.
>
> Afterwards, you can add the path to the library into your
> LD_LIBRARY_PATH, and start Emacs that way.
>
> Thanks a lot!

OK; I'll try that (but probably tomorrow).

> Yes, you should remove it.  I suspect that code was originally added to
> work around the resizing bug I fixed in bug#51679.  Thanks.

Ah, I see.  OK, now removed.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> OK; I'll try that (but probably tomorrow).

BTW, I also see this issue on WebKitGTK 3.34.1 from Fedora, and one of
the developers said it seemed like a regression to him, so I reported a
bug here:

  https://bugs.webkit.org/show_bug.cgi?id=232860

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

[Lars Ingebrigtsen]

Po Lu <luangruo@yahoo.com> writes:

> Can you test the xwidget support using self-built WebKitGTK with
> `-DENABLE_GL=NO' added to your cmake options?
>
> You can follow the instructions here: https://trac.webkit.org/wiki/BuildingGtk.

It finally finished building.  I built from the development (git)
sources...

> Afterwards, you can add the path to the library into your
> LD_LIBRARY_PATH, and start Emacs that way.
>
> Thanks a lot!

... but the recipe for that doesn't have any instructions on how to
install it.  Should I have used a release tarball instead, perhaps?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

[Lars Ingebrigtsen]

Po Lu <luangruo@yahoo.com> writes:

> BTW, I also see this issue on WebKitGTK 3.34.1 from Fedora, and one of
> the developers said it seemed like a regression to him, so I reported a
> bug here:
>
>   https://bugs.webkit.org/show_bug.cgi?id=232860

Ah, OK, then I won't try to debug this further.  😀

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Ah, OK, then I won't try to debug this further.  😀

Yes, and also thanks for all the work you've been doing.  Once the
WebKitGTK problems are resolved, I'm really looking forward to native
video playback in Emacs.

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

[Lars Ingebrigtsen]

Po Lu <luangruo@yahoo.com> writes:

> Yes, and also thanks for all the work you've been doing.  Once the
> WebKitGTK problems are resolved, I'm really looking forward to native
> video playback in Emacs.

I've now added support for this.  Try

(setq shr-use-xwidgets-for-media t)

and use eww to look at a web page with a <video> tag.  It works!  😀
But not on Debian/bullseye.  ☹️  So I've been testing this on Macos, and
it's totally smooth there, as far as I can tell.  (But I've only tested
on a couple of different pages.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> and use eww to look at a web page with a <video> tag.  It works!  😀
> But not on Debian/bullseye.  ☹️  So I've been testing this on Macos, and
> it's totally smooth there, as far as I can tell.  (But I've only tested
> on a couple of different pages.)

It currently creates a temporary file, and I'd like to avoid that.  I
thought about using xwidget-webkit-execute-script to do something like

document.append("<video...>")

but I guess I need to open a "empty" page first?  (I haven't actually
tried.)  Do you have any advice here?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> It currently creates a temporary file, and I'd like to avoid that.  I
> thought about using xwidget-webkit-execute-script to do something like
>
> document.append("<video...>")
>
> but I guess I need to open a "empty" page first?  (I haven't actually
> tried.)  Do you have any advice here?

I think the cleaner solution would be to introduce a function that wraps
`webkit_web_view_load_html', but since the eww code also has to support
macOS, someone here who knows his way around that system will have to
implement it as well.

Also, the WebKit widget loads "about:blank" by default, as long as
RELATED is nil when passed to make-xwidget (this is necessary due to how
WebKit treats related widgets internally), so you shouldn't have to load
an empty page first.

WDYT?

bug#51674: 29.0.50; [PATCH] Fix hang when displaying xwidget script dialog

[Lars Ingebrigtsen]

Po Lu <luangruo@yahoo.com> writes:

> Also, the WebKit widget loads "about:blank" by default, as long as
> RELATED is nil when passed to make-xwidget (this is necessary due to how
> WebKit treats related widgets internally), so you shouldn't have to load
> an empty page first.

Yup; works perfectly.  😀

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no