bug#58271: 29.0.50; [PATCH] Handle sharing Cocoa xwidgets more gracefully

[Sungbin Jo]

2022. 10. 4. 오전 9:34, Po Lu <luangruo@yahoo.com> 작성:

> Thanks; since you seem to be the original author of the xwidget code on
> Mac OS, could you please fix the crash there when an xwidget is deleted
> but remains on-screen?

I’ve continued on this (with some questions) on emacs-devel.

> Some minor formatting comments on the patch below:

Updated patch attached.

>> From: VirtualBuddy <virtualbuddy@monterey-vm.local>
> 
> Is this a real email?

No, it was a mistake. Updated.

>> * etc/NEWS: Mention fix.
> 
> Since this is a bug fix, I see no reason to mention it in NEWS.  In
> addition, even if it warranted a mention, it ought to be placed under
> "Changes in Emacs 29.1 on Non-Free Operating Systems".

I did place it under the “Non-Free Operating Systems” part, but I removed them.

>> * 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.
> 
> Please make sure that each line of the commit message is no longer than
> 64 characters in length.

Done.

>> {
>> -  [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];
>> }
> 
> The "emacswindow" field should actually be named "frame", IMHO.

I’m not sure I follow; that’s exactly the rename I’ve done. Or are you
suggesting that I should use ‘frame’ instead of ‘emacsFrame’? In any
case, I’m fine with either.

>> +    {
>> +      NSTextField *warningLabel = [NSTextField labelWithString:@"Cocoa 
>> Xwidgets do not support sharing widgets."];
> 
> This line is too long.  Please find a way to make it fit in 80 columns.

Done.

>> +      if (xw->xv == xv)
>> +        {
>> +          xw->xv = NULL; /* Now model has no view.  */
>> +        }
> 
> Please remove the unnecessary braces here.

Done.

0001-Handle-sharing-Cocoa-xwidgets-more-gracefully.patch
Description: Binary data