From: Po Lu via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: Lars Ingebrigtsen <larsi@gnus.org>
Cc: 조성빈 <goranmoomin@daum.net>, 58271@debbugs.gnu.org
Subject: bug#58271: 29.0.50; [PATCH] Handle sharing Cocoa xwidgets more gracefully
Date: Tue, 04 Oct 2022 08:34:53 +0800 [thread overview]
Message-ID: <87r0zo1l5e.fsf@yahoo.com> (raw)
In-Reply-To: <87czb86b4g.fsf@gnus.org> (Lars Ingebrigtsen's message of "Mon, 03 Oct 2022 20:00:15 +0200")
Lars Ingebrigtsen <larsi@gnus.org> writes:
> 조성빈 <goranmoomin@daum.net> writes:
>
>> The attached patch fixes this issue by showing a warning label in the place of
>> the xwidget view.
>
> Makes sense to me -- perhaps Po Lu has some comments; added to the CCs.
>
> In any case, it looks like this patch is too large to apply without
> getting a copyright assignment for the FSF first. Would you be willing
> to sign such paperwork?
Thanks. I saw the post to emacs-devel first, and replied without
checking the bug tracker. These were my comments:
조성빈 <goranmoomin@daum.net> writes:
> Hello,
>
> Attached is a patch that improves the handling of sharing Cocoa xwidgets.
>
> 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.
>
> I originally tried to create a bug report with the patch by mailing to
> bug-gnu-emacs@gnu.org, but it seems that the report wasn’t created. As such,
> I’m sending the patch to the emacs-devel list. How should I proceed with this?
>
> Thanks.
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?
Some minor formatting comments on the patch below:
> From: VirtualBuddy <virtualbuddy@monterey-vm.local>
Is this a real email?
> * 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".
> * 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.
> {
> - [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.
> + {
> + 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.
> + if (xw->xv == xv)
> + {
> + xw->xv = NULL; /* Now model has no view. */
> + }
Please remove the unnecessary braces here.
next prev parent reply other threads:[~2022-10-04 0:34 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87r0zo1l5e.fsf@yahoo.com \
--to=bug-gnu-emacs@gnu.org \
--cc=58271@debbugs.gnu.org \
--cc=goranmoomin@daum.net \
--cc=larsi@gnus.org \
--cc=luangruo@yahoo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.