all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Robert Cochran <robert-emacs@cochranmail.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: shanemhansen@gmail.com, 24041@debbugs.gnu.org
Subject: bug#24041: 25.1.50; xwidget + -nw mode gives segfault
Date: Mon, 22 Aug 2016 11:30:15 -0700	[thread overview]
Message-ID: <87k2f87j2g.fsf@cochranmail.com> (raw)
In-Reply-To: <83k2f8c1cd.fsf@gnu.org> (Eli Zaretskii's message of "Mon, 22 Aug 2016 17:41:54 +0300")

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Cochran <robert-emacs@cochranmail.com>
>> Cc: Robert Cochran <robert-emacs@cochranmail.com>,  shanemhansen@gmail.com,  24041@debbugs.gnu.org
>> Date: Sun, 21 Aug 2016 19:12:41 -0700
>> 
>> Anyways, I have a patch, below, that's my first stab at solving the
>> problem. All it does is call `check_x_display_info` with the current
>> frame and allows any resulting error signals to propagate back up.
>> 
>> Probably not the most elegant solution, but I'm not entirely clear what
>> can and can't be done from within the Emacs C core. Suggestions are very
>> welcome.
>
> My only comment is that you could call check_x_display_info with Qnil
> as its argument.

I did think about that. But then it arguably does the wrong thing:
`check_x_display_info` with `Qnil` only signals an error when there have
never been X windows, eg, opening and closing an X window satisfies the
check from then on. It no longer crashes in that instance, but I
personally don't think that's the right behavior; if my starting frame
isn't capable of displaying an xwidget, say so! Hence checking with the
current frame.

That, of course, is just my opinion.

> Otherwise, LGTM, thanks.  And I see nothing inelegant in this patch.

Thanks for your reassurance! My one gripe about this patch is that I
didn't figure out how to kill the buffer after xwidget creation failure
(leaving it seems rather ugly IMO), but I just now realized what I can
do. As long as it's not considered wrong to kill a mode's buffer on
error, would you also consider this patch to go along with it?

-----


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Kill webkit browser window if error occurs --]
[-- Type: text/x-patch, Size: 1499 bytes --]

From e58ed09121969febde5bfc2206c14f4a7806c323 Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git@cochranmail.com>
Date: Mon, 22 Aug 2016 11:21:01 -0700
Subject: [PATCH] Kill the webkit browser buffer if an error occurs

* lisp/xwidget.el (xwidget-webkit-new-session): kill webkit buffer on
  error
---
 lisp/xwidget.el | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lisp/xwidget.el b/lisp/xwidget.el
index 7a0ca8b..e925c9a 100644
--- a/lisp/xwidget.el
+++ b/lisp/xwidget.el
@@ -427,11 +427,16 @@ xwidget-webkit-new-session
        xw)
     (setq xwidget-webkit-last-session-buffer (switch-to-buffer
                                               (get-buffer-create bufname)))
-    (insert " 'a' adjusts the xwidget size.")
-    (setq xw (xwidget-insert 1 'webkit  bufname 1000 1000))
-    (xwidget-put xw 'callback 'xwidget-webkit-callback)
-    (xwidget-webkit-mode)
-    (xwidget-webkit-goto-uri (xwidget-webkit-last-session) url)))
+    (condition-case err
+        (progn
+          (insert " 'a' adjusts the xwidget size.")
+          (setq xw (xwidget-insert 1 'webkit  bufname 1000 1000))
+          (xwidget-put xw 'callback 'xwidget-webkit-callback)
+          (xwidget-webkit-mode)
+          (xwidget-webkit-goto-uri (xwidget-webkit-last-session) url))
+      ;; On error, remove webkit buffer and resignal
+      (error (kill-buffer bufname)
+             (signal (car err) (cdr err))))))
 
 
 (defun xwidget-webkit-goto-url (url)
-- 
2.7.4


[-- Attachment #3: Type: text/plain, Size: 188 bytes --]

-----

Otherwise, with or without the new patch, I have no issues if you have
no issues.

Thanks,
-- 
~Robert Cochran

GPG Fingerprint - E778 2DD4 FEA6 6A68 6F26  AD2D E5C3 EB36 4886 8871

  reply	other threads:[~2016-08-22 18:30 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-20 16:04 bug#24041: 25.1.50; xwidget + -nw mode gives segfault Shane Hansen
2016-08-19 18:36 ` Robert Cochran
2016-08-20  7:32   ` Eli Zaretskii
2016-08-20 21:33     ` Robert Cochran
2016-08-21  2:40       ` Eli Zaretskii
2016-08-22  2:12         ` Robert Cochran
2016-08-22 14:41           ` Eli Zaretskii
2016-08-22 18:30             ` Robert Cochran [this message]
2016-08-22 18:49               ` Eli Zaretskii
2016-08-22 19:52                 ` joakim
2016-08-22 20:28                   ` Robert Cochran
2016-08-23  2:40                     ` Eli Zaretskii
2019-08-28 20:04 ` Lars Ingebrigtsen
2019-08-29 11:00   ` Robert Pluim

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=87k2f87j2g.fsf@cochranmail.com \
    --to=robert-emacs@cochranmail.com \
    --cc=24041@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    --cc=shanemhansen@gmail.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.