all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#58271: 29.0.50; [PATCH] Handle sharing Cocoa xwidgets more gracefully
@ 2022-10-03  9:32 조성빈
  2022-10-03 18:00 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 15+ messages in thread
From: 조성빈 @ 2022-10-03  9:32 UTC (permalink / raw)
  To: 58271

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

Severity: normal
Tags: patch

The previous implementation of Cocoa xwidgets poorly handled cases where the
user tried to share xwidgets between multiple views (by spamming messages to
*Messages* on every redisplay, and not drawing anything on the second xwidget).
The attached patch fixes this issue by showing a warning label in the place of
the xwidget view.


[-- Attachment #2: 0001-Handle-sharing-Cocoa-xwidgets-more-gracefully.patch --]
[-- Type: application/octet-stream, Size: 12525 bytes --]

From ea24ca5d49bb0c1d09be98ab032bbc50668ae627 Mon Sep 17 00:00:00 2001
From: VirtualBuddy <virtualbuddy@monterey-vm.local>
Date: Mon, 3 Oct 2022 17:32:45 +0900
Subject: [PATCH] Handle sharing Cocoa xwidgets more gracefully

* etc/NEWS: Mention fix.
* src/nsxwidget.h: Remove now-unused NSView subclasses and functions.
* src/nsxwidget.m:
([XwWebView mouseDown:], [XwWebView mouseUp:], [XwWebView keyDown:])
([XwWebView userContentController:didReceiveScriptMessage:]): Rename field of
xwidget_view from emacswindow to emacsFrame to better match emacs terminology.
(nsxwidget_init, nsxwidget_resize_view, nsxwidget_move_widget_in_view):
Simplify logic by removing field xwWindow and using the xvWindow as the
container.
(nsxwidget_resize, XwWindow, XvWindow): Remove now-unused code.
(nsxwidget_init_view, nsxwidget_delete_view): Handle creating non-primary
xwidget views.
(nsxwidget_show_view, nsxwidget_hide_view): Remove poor hack to hide views.
* src/xwidget.c (xwidget_init_view): Update formatting.
(x_draw_xwidget_glyph_string): Handle displaying non-primary xwidget views and
remove previous message warning.
(Fxwidget_resize): Remove useless call to nsxwidget_resize, as the subsequent
redisplay handles them via nsxwidget_resize_view.
* src/xwidget.h (struct xwidget): Remove field xwWindow and update comments
to be more accurate.
(struct xwidget_view): Add field xvWidget and rename field emacswindow to
emacsFrame to better match emacs terminology.
---
 etc/NEWS        |  8 +++++
 src/nsxwidget.h | 14 --------
 src/nsxwidget.m | 87 ++++++++++++++++++++-----------------------------
 src/xwidget.c   | 11 ++-----
 src/xwidget.h   | 15 ++++-----
 5 files changed, 53 insertions(+), 82 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index ec23f10b1f..ec85d0ba41 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -4016,6 +4016,14 @@ has built-in support for displaying BMP images.
 ---
 *** 'process-attributes' is now implemented.
 
+** macOS
+---
+*** Attempts to share an xwidget is now handled more gracefully.
+Attempts to share an xwidget between multiple views now results in
+views other than the primary view displaying warnings.  If the primary
+view disappears, one of the left views becomes the primary view that
+displays the widget.
+
 \f
 ----------------------------------------------------------------------
 This file is part of GNU Emacs.
diff --git a/src/nsxwidget.h b/src/nsxwidget.h
index 666509744a..f7c744d405 100644
--- a/src/nsxwidget.h
+++ b/src/nsxwidget.h
@@ -42,26 +42,12 @@ #define NSXWIDGET_H_INCLUDED
 
 /* Functions for xwidget model.  */
 
-#ifdef __OBJC__
-@interface XwWindow : NSView
-@property struct xwidget *xw;
-@end
-#endif
-
 void nsxwidget_init (struct xwidget *xw);
 void nsxwidget_kill (struct xwidget *xw);
-void nsxwidget_resize (struct xwidget *xw);
 Lisp_Object nsxwidget_get_size (struct xwidget *xw);
 
 /* Functions for xwidget view.  */
 
-#ifdef __OBJC__
-@interface XvWindow : NSView
-@property struct xwidget *xw;
-@property struct xwidget_view *xv;
-@end
-#endif
-
 void nsxwidget_init_view (struct xwidget_view *xv,
                           struct xwidget *xww,
                           struct glyph_string *s,
diff --git a/src/nsxwidget.m b/src/nsxwidget.m
index be0eba0bcb..da4069e2f1 100644
--- a/src/nsxwidget.m
+++ b/src/nsxwidget.m
@@ -199,13 +199,13 @@ - (void)webView:(WKWebView *)webView
 
 - (void)mouseDown:(NSEvent *)event
 {
-  [self.xw->xv->emacswindow mouseDown:event];
+  [self.xw->xv->emacsFrame mouseDown:event];
   [super mouseDown:event];
 }
 
 - (void)mouseUp:(NSEvent *)event
 {
-  [self.xw->xv->emacswindow mouseUp:event];
+  [self.xw->xv->emacsFrame mouseUp:event];
   [super mouseUp:event];
 }
 
@@ -220,15 +220,15 @@ - (void)keyDown:(NSEvent *)event
   Lisp_Object val = buffer_local_value (var, Fcurrent_buffer ());
   if (!EQ (val, Qunbound) && !EQ (val, Qnil))
     {
-      [self.window makeFirstResponder:self.xw->xv->emacswindow];
-      [self.xw->xv->emacswindow keyDown:event];
+      [self.window makeFirstResponder:self.xw->xv->emacsFrame];
+      [self.xw->xv->emacsFrame keyDown:event];
       return;
     }
 
   /* Emacs handles keyboard events when javascript is blocked.  */
   if ([self.urlScriptBlocked[self.URL] boolValue])
     {
-      [self.xw->xv->emacswindow keyDown:event];
+      [self.xw->xv->emacsFrame keyDown:event];
       return;
     }
 
@@ -237,13 +237,13 @@ - (void)keyDown:(NSEvent *)event
       if (error)
         {
           NSLog (@"xwHasFocus: %@", error);
-          [self.xw->xv->emacswindow keyDown:event];
+          [self.xw->xv->emacsFrame keyDown:event];
         }
       else if (result)
         {
           NSNumber *hasFocus = result; /* __NSCFBoolean */
           if (!hasFocus.boolValue)
-            [self.xw->xv->emacswindow keyDown:event];
+            [self.xw->xv->emacsFrame keyDown:event];
           else
             [super keyDown:event];
         }
@@ -291,7 +291,7 @@ - (void)userContentController:(WKUserContentController *)userContentController
     {
       /* Just give up focus, no relay "C-g" to emacs, another "C-g"
          follows will be handled by emacs.  */
-      [self.window makeFirstResponder:self.xw->xv->emacswindow];
+      [self.window makeFirstResponder:self.xw->xv->emacsFrame];
     }
 }
 
@@ -437,16 +437,10 @@ - (void)userContentController:(WKUserContentController *)userContentController
     }];
 }
 
-/* Window containing an xwidget.  */
-
-@implementation XwWindow
-- (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);
@@ -454,9 +448,6 @@ - (BOOL)isFlipped { return YES; }
                    initWithFrame:rect
                    configuration:[[WKWebViewConfiguration alloc] init]
                          xwidget:xw];
-  xw->xwWindow = [[XwWindow alloc]
-                   initWithFrame:rect];
-  [xw->xwWindow addSubview:xw->xwWidget];
   xw->xv = NULL; /* for 1 to 1 relationship of webkit2.  */
   unblock_input ();
 }
@@ -481,22 +472,10 @@ - (BOOL)isFlipped { return YES; }
       [((XwWebView *) xw->xwWidget).urlScriptBlocked release];
       [xw->xwWidget removeFromSuperviewWithoutNeedingDisplay];
       [xw->xwWidget release];
-      [xw->xwWindow removeFromSuperviewWithoutNeedingDisplay];
-      [xw->xwWindow release];
       xw->xwWidget = nil;
     }
 }
 
-void
-nsxwidget_resize (struct xwidget *xw)
-{
-  if (xw->xwWidget)
-    {
-      [xw->xwWindow setFrameSize:NSMakeSize(xw->width, xw->height)];
-      [xw->xwWidget setFrameSize:NSMakeSize(xw->width, xw->height)];
-    }
-}
-
 Lisp_Object
 nsxwidget_get_size (struct xwidget *xw)
 {
@@ -504,12 +483,6 @@ - (BOOL)isFlipped { return YES; }
                  xw->xwWidget.frame.size.height);
 }
 
-/* Xwidget view, macOS Cocoa part.  */
-
-@implementation XvWindow : NSView
-- (BOOL)isFlipped { return YES; }
-@end
-
 void
 nsxwidget_init_view (struct xwidget_view *xv,
                      struct xwidget *xw,
@@ -526,16 +499,27 @@ - (BOOL)isFlipped { return YES; }
   xv->clip_top = 0;
   xv->clip_bottom = xw->height;
 
-  xv->xvWindow = [[XvWindow alloc]
+  xv->xvWindow = [[NSView alloc]
                    initWithFrame:NSMakeRect (x, y, xw->width, xw->height)];
-  xv->xvWindow.xw = xw;
-  xv->xvWindow.xv = xv;
 
-  xw->xv = xv; /* For 1 to 1 relationship of webkit2.  */
-  [xv->xvWindow addSubview:xw->xwWindow];
+  if (!xw->xv)
+    {
+      xw->xv = xv; /* For 1 to 1 relationship of webkit2.  */
+      /* This seems like it should be retained, but tests show otherwise.  */
+      xv->xvWidget = xw->xwWidget;
+    }
+  else
+    {
+      NSTextField *warningLabel = [NSTextField labelWithString:@"Cocoa Xwidgets do not support sharing widgets."];
+      [warningLabel setFrameSize:NSMakeSize (xw->width, xw->height)];
+      warningLabel.drawsBackground = YES;
+      warningLabel.backgroundColor = [NSColor textBackgroundColor];
+      xv->xvWidget = warningLabel;
+    }
 
-  xv->emacswindow = FRAME_NS_VIEW (s->f);
-  [xv->emacswindow addSubview:xv->xvWindow];
+  [xv->xvWindow addSubview:xv->xvWidget];
+  xv->emacsFrame = FRAME_NS_VIEW (s->f);
+  [xv->emacsFrame addSubview:xv->xvWindow];
 }
 
 void
@@ -544,8 +528,10 @@ - (BOOL)isFlipped { return YES; }
   if (!EQ (xv->model, Qnil))
     {
       struct xwidget *xw = XXWIDGET (xv->model);
-      [xw->xwWindow removeFromSuperviewWithoutNeedingDisplay];
-      xw->xv = NULL; /* Now model has no view.  */
+      if (xw->xv == xv)
+        {
+          xw->xv = NULL; /* Now model has no view.  */
+        }
     }
   [xv->xvWindow removeFromSuperviewWithoutNeedingDisplay];
   [xv->xvWindow release];
@@ -555,21 +541,21 @@ - (BOOL)isFlipped { return YES; }
 nsxwidget_show_view (struct xwidget_view *xv)
 {
   xv->hidden = NO;
-  [xv->xvWindow setFrameOrigin:NSMakePoint(xv->x + xv->clip_left,
-                                           xv->y + xv->clip_top)];
+  [xv->xvWindow setHidden:NO];
 }
 
 void
 nsxwidget_hide_view (struct xwidget_view *xv)
 {
   xv->hidden = YES;
-  [xv->xvWindow setFrameOrigin:NSMakePoint(10000, 10000)];
+  [xv->xvWindow setHidden:YES];
 }
 
 void
 nsxwidget_resize_view (struct xwidget_view *xv, int width, int height)
 {
-  [xv->xvWindow setFrameSize:NSMakeSize(width, height)];
+  [xv->xvWindow setFrameSize:NSMakeSize (width, height)];
+  [xv->xvWidget setFrameSize:NSMakeSize (width, height)];
 }
 
 void
@@ -582,8 +568,7 @@ - (BOOL)isFlipped { return YES; }
 void
 nsxwidget_move_widget_in_view (struct xwidget_view *xv, int x, int y)
 {
-  struct xwidget *xww = xv->xvWindow.xw;
-  [xww->xwWindow setFrameOrigin:NSMakePoint (x, y)];
+  [xv->xvWidget setFrameOrigin:NSMakePoint (x, y)];
 }
 
 void
diff --git a/src/xwidget.c b/src/xwidget.c
index 8bdfab02fd..013b773a90 100644
--- a/src/xwidget.c
+++ b/src/xwidget.c
@@ -2762,7 +2762,7 @@ xwidget_init_view (struct xwidget *xww,
   xv->just_resized = false;
 #elif defined NS_IMPL_COCOA
   nsxwidget_init_view (xv, xww, s, x, y);
-  nsxwidget_resize_view(xv, xww->width, xww->height);
+  nsxwidget_resize_view (xv, xww->width, xww->height);
 #endif
 
   return xv;
@@ -2794,7 +2794,7 @@ x_draw_xwidget_glyph_string (struct glyph_string *s)
 
   xv->just_resized = false;
 #elif defined NS_IMPL_COCOA
-  if (!xv)
+  if (!xv || xww->xv != xv)
     {
       /* Enforce 1 to 1, model and view for macOS Cocoa webkit2.  */
       if (xww->xv)
@@ -2805,11 +2805,6 @@ x_draw_xwidget_glyph_string (struct glyph_string *s)
               XSETXWIDGET_VIEW (xvl, xww->xv);
               Fdelete_xwidget_view (xvl);
             }
-          else
-            {
-              message ("You can't share an xwidget (webkit2) among windows.");
-              return;
-            }
         }
       xv = xwidget_init_view (xww, s, x, y);
     }
@@ -3243,8 +3238,6 @@ DEFUN ("xwidget-resize", Fxwidget_resize, Sxwidget_resize, 3, 3, 0,
 
       gtk_widget_queue_allocate (GTK_WIDGET (xw->widget_osr));
     }
-#elif defined NS_IMPL_COCOA
-  nsxwidget_resize (xw);
 #endif
   unblock_input ();
 
diff --git a/src/xwidget.h b/src/xwidget.h
index 502beb6765..3943148e3f 100644
--- a/src/xwidget.h
+++ b/src/xwidget.h
@@ -81,16 +81,13 @@ #define XWIDGET_H_INCLUDED
   guint hit_result;
 #elif defined (NS_IMPL_COCOA)
 # ifdef __OBJC__
-  /* For offscreen widgets, unused if not osr.  */
+  /* The primary widget.  */
   NSView *xwWidget;
-  XwWindow *xwWindow;
 
-  /* Used only for xwidget types (such as webkit2) enforcing 1 to 1
-     relationship between model and view.  */
+  /* The current view that displays the primary widget, if exists.  */
   struct xwidget_view *xv;
 # else
   void *xwWidget;
-  void *xwWindow;
   struct xwidget_view *xv;
 # endif
 #endif
@@ -136,11 +133,13 @@ #define XWIDGET_H_INCLUDED
   int just_resized;
 #elif defined (NS_IMPL_COCOA)
 # ifdef __OBJC__
-  XvWindow *xvWindow;
-  NSView *emacswindow;
+  NSView *xvWindow;
+  NSView *xvWidget;
+  NSView *emacsFrame;
 # else
   void *xvWindow;
-  void *emacswindow;
+  void *xvWidget;
+  void *emacsFrame;
 # endif
 #endif
 
-- 
2.37.0 (Apple Git-136)


^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-11-26  5:54 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-03  9:32 bug#58271: 29.0.50; [PATCH] Handle sharing Cocoa xwidgets more gracefully 조성빈
2022-10-03 18:00 ` Lars Ingebrigtsen
2022-10-03 23:01   ` goranmoomin
2022-10-03 23:35     ` Lars Ingebrigtsen
2022-10-04  0:01       ` Sungbin Jo
2022-10-04  4:42       ` Stefan Kangas
2022-10-04  6:48       ` Eli Zaretskii
2022-10-04  7:25         ` Sungbin Jo
2022-11-11 13:27           ` Stefan Kangas
2022-10-04  0:34   ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-10-04  2:03     ` Sungbin Jo
2022-11-25  1:26       ` Stefan Kangas
2022-11-25  2:55         ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-11-26  5:31           ` 조성빈
2022-11-26  5:54             ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.