* bug#25778: 25.1; [PATCH] Drastically simplify xdg-open check @ 2017-02-17 18:16 Vasilij Schneidermann 2017-02-17 18:38 ` Eli Zaretskii 2017-03-10 3:13 ` Paul Eggert 0 siblings, 2 replies; 11+ messages in thread From: Vasilij Schneidermann @ 2017-02-17 18:16 UTC (permalink / raw) To: 25778 [-- Attachment #1: Type: text/plain, Size: 710 bytes --] A user on #emacs brought it to my attention that the xdg-open detection for browse-url is more complex than it should be and fails detecting their Linux desktop. Looking at the definition of `browse-url-can-use-xdg-open' reveals that it replicates the desktop environment check as done by `xdg-open` (which is done to detect the DE-specific default browser setting, not to prevent people not using a popular desktop environment from using xdg-open). As enumerating all possible kinds of Linux desktop is a futile endeavor, I've completely eliminated this check from it so that the only ones left are whether we're in a graphical session (by checking $DISPLAY) and whether `xdg-open` has been found on $PATH. [-- Attachment #2: 0001-Drastically-simplify-check-for-xdg-open.patch --] [-- Type: text/x-patch, Size: 2714 bytes --] From 6c99e7ace3110b73a5b212203acc728165be092b Mon Sep 17 00:00:00 2001 From: Vasilij Schneidermann <v.schneidermann@gmail.com> Date: Fri, 17 Feb 2017 19:08:54 +0100 Subject: [PATCH] Drastically simplify check for xdg-open * browse-url.el (browse-url-can-use-xdg-open): Simplify xdg-open check --- lisp/net/browse-url.el | 34 +++------------------------------- 1 file changed, 3 insertions(+), 31 deletions(-) diff --git a/lisp/net/browse-url.el b/lisp/net/browse-url.el index a7c879cbfb..bf7561306c 100644 --- a/lisp/net/browse-url.el +++ b/lisp/net/browse-url.el @@ -45,7 +45,7 @@ ;; browse-url-generic arbitrary ;; browse-url-default-windows-browser MS-Windows browser ;; browse-url-default-macosx-browser macOS browser -;; browse-url-xdg-open Free Desktop xdg-open on Gnome, KDE, Xfce4, LXDE +;; browse-url-xdg-open Free Desktop xdg-open ;; browse-url-kde KDE konqueror (kfm) ;; browse-url-elinks Elinks Don't know (tried with 0.12.GIT) @@ -944,36 +944,8 @@ instead of `browse-url-new-window-flag'." (defun browse-url-can-use-xdg-open () "Return non-nil if the \"xdg-open\" program can be used. -xdg-open is a desktop utility that calls your preferred web browser. -This requires you to be running either Gnome, KDE, Xfce4 or LXDE." - (and (getenv "DISPLAY") - (executable-find "xdg-open") - ;; xdg-open may call gnome-open and that does not wait for its child - ;; to finish. This child may then be killed when the parent dies. - ;; Use nohup to work around. See bug#7166, bug#8917, bug#9779 and - ;; http://lists.gnu.org/archive/html/emacs-devel/2009-07/msg00279.html - (executable-find "nohup") - (or (getenv "GNOME_DESKTOP_SESSION_ID") - ;; GNOME_DESKTOP_SESSION_ID is deprecated, check on Dbus also. - (condition-case nil - (eq 0 (call-process - "dbus-send" nil nil nil - "--dest=org.gnome.SessionManager" - "--print-reply" - "/org/gnome/SessionManager" - "org.gnome.SessionManager.CanShutdown")) - (error nil)) - (equal (getenv "KDE_FULL_SESSION") "true") - (condition-case nil - (eq 0 (call-process - "/bin/sh" nil nil nil - "-c" - ;; FIXME use string-match rather than grep. - "xprop -root _DT_SAVE_MODE|grep xfce4")) - (error nil)) - (member (getenv "DESKTOP_SESSION") '("LXDE" "Lubuntu")) - (equal (getenv "XDG_CURRENT_DESKTOP") "LXDE")))) - +xdg-open is a desktop utility that calls your preferred web browser." + (and (getenv "DISPLAY") (executable-find "xdg-open"))) ;;;###autoload (defun browse-url-xdg-open (url &optional ignored) -- 2.11.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* bug#25778: 25.1; [PATCH] Drastically simplify xdg-open check 2017-02-17 18:16 bug#25778: 25.1; [PATCH] Drastically simplify xdg-open check Vasilij Schneidermann @ 2017-02-17 18:38 ` Eli Zaretskii 2017-02-17 18:55 ` Vasilij Schneidermann 2017-03-10 3:13 ` Paul Eggert 1 sibling, 1 reply; 11+ messages in thread From: Eli Zaretskii @ 2017-02-17 18:38 UTC (permalink / raw) To: Vasilij Schneidermann; +Cc: 25778 > From: Vasilij Schneidermann <v.schneidermann@gmail.com> > Date: Fri, 17 Feb 2017 19:16:48 +0100 > > A user on #emacs brought it to my attention that the xdg-open detection > for browse-url is more complex than it should be and fails detecting > their Linux desktop. Looking at the definition of > `browse-url-can-use-xdg-open' reveals that it replicates the desktop > environment check as done by `xdg-open` (which is done to detect the > DE-specific default browser setting, not to prevent people not using a > popular desktop environment from using xdg-open). As enumerating all > possible kinds of Linux desktop is a futile endeavor, I've completely > eliminated this check from it so that the only ones left are whether > we're in a graphical session (by checking $DISPLAY) and whether > `xdg-open` has been found on $PATH. First, checking $DISPLAY doesn't yet mean you are in a GUI frame. We have display-graphic-p for that. And second, I don't understand what will happen with all the bugs mentioned in the comments if we remove the code that was supposed to avoid hitting them. Maybe I'm missing something obvious. ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#25778: 25.1; [PATCH] Drastically simplify xdg-open check 2017-02-17 18:38 ` Eli Zaretskii @ 2017-02-17 18:55 ` Vasilij Schneidermann 2017-02-18 3:39 ` Glenn Morris 2017-02-18 7:58 ` Eli Zaretskii 0 siblings, 2 replies; 11+ messages in thread From: Vasilij Schneidermann @ 2017-02-17 18:55 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 25778 > First, checking $DISPLAY doesn't yet mean you are in a GUI frame. We > have display-graphic-p for that. The assumption that you need to be in a GUI frame is incorrect. If I run `emacs -nw` in a graphical terminal emulator, why would that prevent me from opening a graphical browser? The only way I see to detect whether a graphical browser can be opened is by checking for X with $DISPLAY. > And second, I don't understand what will happen with all the bugs > mentioned in the comments if we remove the code that was supposed to > avoid hitting them. Maybe I'm missing something obvious. OK, I'll go through them all for completeness' sake: - bug#7166: This one arises from using /bin/sh in combination with `nohup` for launching xdg-open, resulting in a lack of shell quoting. As `xdg-open` is launched directly, it's no longer relevant and only mentioned because of `nohup` (which is no longer used either). - bug#8917: The old usage of `call-process` has made `nohup` obsolete. Irrelevant as `nohup` isn't used any longer. - bug#9779: Extension of the now removed xdg-open DE detection logic, together with some discussion whether nohup is still necessary. Some voices in favor of not doing any unnecessary detection. - The linked emacs-devel thread: Sighting of a possible bug in gnome-open. As `nohup` is no longer used, there's no way this could have been worked around with its usage Summary: All linked bugs are related to `nohup` which is only checked for, but not actually used. Therefore they can be disregarded. Do you have any actual objections based on a situation where one would *not* want to use xdg-open, even though it's installed and the user in a X11 session? ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#25778: 25.1; [PATCH] Drastically simplify xdg-open check 2017-02-17 18:55 ` Vasilij Schneidermann @ 2017-02-18 3:39 ` Glenn Morris 2017-02-18 3:46 ` Glenn Morris 2017-02-18 7:58 ` Eli Zaretskii 1 sibling, 1 reply; 11+ messages in thread From: Glenn Morris @ 2017-02-18 3:39 UTC (permalink / raw) To: Vasilij Schneidermann; +Cc: 25778 The area seems a mess, but: 1) nohup was removed from browse-url-xdg-open in 1c0f1a199ca, with no explanation and as part of a change that seems unrelated. 2) In https://debbugs.gnu.org/9779#20, the author of that change said I note that since 22-Jun-2011 browse-url-xdg-open no longer uses nohup. This is either a mistake on my part (I didn't read the comment in browse-url-can-use-xdg-open) or it is indeed unnecessary and therefore isn't a requirement for xdg-open. 3) In reply, Jan said in https://debbugs.gnu.org/9779#29 See discussion at http://lists.gnu.org/archive/html/emacs-devel/2009-07/msg00279.html. nohup is still needed for xdg-open. And AFAICS that was the last comment about nohup. No-one ever seems to have followed up Jan's comment. ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#25778: 25.1; [PATCH] Drastically simplify xdg-open check 2017-02-18 3:39 ` Glenn Morris @ 2017-02-18 3:46 ` Glenn Morris 2017-02-18 8:55 ` Vasilij Schneidermann 0 siblings, 1 reply; 11+ messages in thread From: Glenn Morris @ 2017-02-18 3:46 UTC (permalink / raw) To: Vasilij Schneidermann; +Cc: 25778 PS Maybe you are just saying that since no-one (?) complained since then, nohup can't have been needed after all. It would be better to test it, though. It seems difficult to test all the possibilities for xdg-open. Ref https://debbugs.gnu.org/25234 ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#25778: 25.1; [PATCH] Drastically simplify xdg-open check 2017-02-18 3:46 ` Glenn Morris @ 2017-02-18 8:55 ` Vasilij Schneidermann 2017-02-19 0:31 ` Glenn Morris 0 siblings, 1 reply; 11+ messages in thread From: Vasilij Schneidermann @ 2017-02-18 8:55 UTC (permalink / raw) To: Glenn Morris; +Cc: 25778 > PS Maybe you are just saying that since no-one (?) complained since > then, nohup can't have been needed after all. Pretty much. Note that `gnome-open` belongs to GNOME 2 which last release has been in 2010. Only Debian Squeeze is offering it in its package archives and as that one has been EOL'd, I don't see any reason to pursue it any further. If `gvfs-open` exhibits the same behavior, maybe, but I still consider it upstream's fault if they have weird open behavior, not an area of Emacs to clumsily work around. ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#25778: 25.1; [PATCH] Drastically simplify xdg-open check 2017-02-18 8:55 ` Vasilij Schneidermann @ 2017-02-19 0:31 ` Glenn Morris 2017-02-19 13:06 ` Vasilij Schneidermann 0 siblings, 1 reply; 11+ messages in thread From: Glenn Morris @ 2017-02-19 0:31 UTC (permalink / raw) To: Vasilij Schneidermann; +Cc: 25778 Vasilij Schneidermann wrote: > If `gvfs-open` exhibits the same behavior, It does. https://bugzilla.gnome.org/show_bug.cgi?id=652262 And https://debbugs.gnu.org/25234, as previously mentioned. > maybe, but I still consider it upstream's fault As you can see from that above report, they don't show any signs of fixing it. > if they have weird open behavior, not an area of Emacs to clumsily > work around. The people who implemented support for xdg-open in Emacs disagreed. ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#25778: 25.1; [PATCH] Drastically simplify xdg-open check 2017-02-19 0:31 ` Glenn Morris @ 2017-02-19 13:06 ` Vasilij Schneidermann 0 siblings, 0 replies; 11+ messages in thread From: Vasilij Schneidermann @ 2017-02-19 13:06 UTC (permalink / raw) To: Glenn Morris; +Cc: 25778 > As you can see from that above report, they don't show any signs of > fixing it. Well, fine. I'm sorry for not noticing a five year old piece of code that got mistakenly removed without anyone complaining. What I don't understand is why my patch (removing IMHO unnecessary DE detection) is related to using nohup to work around a problem with xdg-open and gnome-open/gvfs-open. If you consider fixing the nohup problem a prerequisite before this patch can be applied, OK, do that then, I can wait until that's done and recommence discussion on the other issue. That aside, I cannot reproduce any weird behavior, be it with or without nohup and gvfs-open. Both `(call-process "gvfs-open" nil 0 nil "foo.pdf")` and `(call-process "nohup" nil 0 nil "gvfs-open" "foo.pdf")` spawn my PDF reader successfully and don't do anything weird after closing it. Same if I pass "http://ix.de/" as last argument. I therefore consider the nohup issue a case of cargo-culting and would like to drop its discussion entirely as it does not help with the current issue. ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#25778: 25.1; [PATCH] Drastically simplify xdg-open check 2017-02-17 18:55 ` Vasilij Schneidermann 2017-02-18 3:39 ` Glenn Morris @ 2017-02-18 7:58 ` Eli Zaretskii 2017-02-18 8:50 ` Vasilij Schneidermann 1 sibling, 1 reply; 11+ messages in thread From: Eli Zaretskii @ 2017-02-18 7:58 UTC (permalink / raw) To: Vasilij Schneidermann; +Cc: 25778 > Date: Fri, 17 Feb 2017 19:55:14 +0100 > From: Vasilij Schneidermann <v.schneidermann@gmail.com> > Cc: 25778@debbugs.gnu.org > > > First, checking $DISPLAY doesn't yet mean you are in a GUI frame. We > > have display-graphic-p for that. > > The assumption that you need to be in a GUI frame is incorrect. If I > run `emacs -nw` in a graphical terminal emulator, why would that prevent > me from opening a graphical browser? The only way I see to detect > whether a graphical browser can be opened is by checking for X with > $DISPLAY. In that case, maybe we shouldn't test $DISPLAY at all? Why restrict this to X if xdg-open is already tested for availability? > Do you have any actual objections based on a situation where one would > *not* want to use xdg-open, even though it's installed and the user in a > X11 session? See Glenn's response about that. I think we should be more careful with these issues, and at least do some more research of them before we can be sure they are no longer relevant. If you could follow up on that, it would be great. Thanks. ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#25778: 25.1; [PATCH] Drastically simplify xdg-open check 2017-02-18 7:58 ` Eli Zaretskii @ 2017-02-18 8:50 ` Vasilij Schneidermann 0 siblings, 0 replies; 11+ messages in thread From: Vasilij Schneidermann @ 2017-02-18 8:50 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 25778 > In that case, maybe we shouldn't test $DISPLAY at all? Why restrict > this to X if xdg-open is already tested for availability? To quote its man page: "xdg-open is for use inside a desktop session only.". If you attempt using it outside one (like, on the Linux console), M-x browse-url fails silently. ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#25778: 25.1; [PATCH] Drastically simplify xdg-open check 2017-02-17 18:16 bug#25778: 25.1; [PATCH] Drastically simplify xdg-open check Vasilij Schneidermann 2017-02-17 18:38 ` Eli Zaretskii @ 2017-03-10 3:13 ` Paul Eggert 1 sibling, 0 replies; 11+ messages in thread From: Paul Eggert @ 2017-03-10 3:13 UTC (permalink / raw) To: Vasilij Schneidermann; +Cc: 25778-done [-- Attachment #1: Type: text/plain, Size: 765 bytes --] Thanks for the bug report. I installed the attached somewhat-more-ambitious patch that works along the line that you suggested. Although every heuristic in this area will be wrong sometimes, I think nowadays we're probably better off simply using xdg-open if it's available and if there's a display. It turns out that nohup is ineffective on xdg-open, as xdg-open's child does a 'signal (SIGHUP, SIG_DFL)' before it execs (!). This may help to explain why nobody noticed when Emacs stopped using nohup. Although nohup may have been needed for ancient GNOME versions, I don't think we need to worry about these old GNOME versions in future Emacs releases. I'm closing the bug report as I think the bug is fixed. We can reopen it if I'm wrong. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Simplify-checks-for-xdg-open-and-xdg-email.patch --] [-- Type: text/x-diff; name="0001-Simplify-checks-for-xdg-open-and-xdg-email.patch", Size: 5679 bytes --] From 095e7d126ee1fc56cb1ec0f9ab1a9cdd6417a232 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Thu, 9 Mar 2017 19:01:19 -0800 Subject: [PATCH] Simplify checks for xdg-open and xdg-email browse-url's xdg-open detection was too picky on some GNU/Linux desktops; see Bug#25778. Simplify the code by assuming xdg-open works if it is executable, as nowadays this is more likely to be correct than trying to use heuristics from a few years ago. Don't test for nohup: it is ineffective nowadays, as xdg-open's child uses the default action for SIGHUP even if xdg-open's invoker ignores SIGHUP. While we're at it, allow for Wayland here, as "emacs -nw" might be running in a non-X Wayland terminal. * lisp/mail/emacsbug.el (report-emacs-bug-can-use-xdg-email): * lisp/net/browse-url.el (browse-url-can-use-xdg-open): Simplify to a test for DISPLAY and whether the helper program is executable. Allow WAYLAND_DISPLAY as an option. --- lisp/mail/emacsbug.el | 29 ++++------------------------- lisp/net/browse-url.el | 40 +++++++++------------------------------- 2 files changed, 13 insertions(+), 56 deletions(-) diff --git a/lisp/mail/emacsbug.el b/lisp/mail/emacsbug.el index ecb7db6..c1aec69 100644 --- a/lisp/mail/emacsbug.el +++ b/lisp/mail/emacsbug.el @@ -77,33 +77,12 @@ report-emacs-bug-can-use-osx-open (equal (executable-find "open") "/usr/bin/open") (memq system-type '(darwin)))) -;; FIXME this duplicates much of the logic from browse-url-can-use-xdg-open. (defun report-emacs-bug-can-use-xdg-email () "Return non-nil if the \"xdg-email\" command can be used. -xdg-email is a desktop utility that calls your preferred mail client. -This requires you to be running either Gnome, KDE, or Xfce4." - (and (getenv "DISPLAY") - (executable-find "xdg-email") - (or (getenv "GNOME_DESKTOP_SESSION_ID") - ;; GNOME_DESKTOP_SESSION_ID is deprecated, check on Dbus also. - (condition-case nil - (eq 0 (call-process - "dbus-send" nil nil nil - "--dest=org.gnome.SessionManager" - "--print-reply" - "/org/gnome/SessionManager" - "org.gnome.SessionManager.CanShutdown")) - (error nil)) - (equal (getenv "KDE_FULL_SESSION") "true") - ;; FIXME? browse-url-can-use-xdg-open also accepts LXDE. - ;; Is that no good here, or just overlooked? - (condition-case nil - (eq 0 (call-process - "/bin/sh" nil nil nil - "-c" - ;; FIXME use string-match rather than grep. - "xprop -root _DT_SAVE_MODE|grep xfce4")) - (error nil))))) +xdg-email is a desktop utility that calls your preferred mail client." + (and ;; See browse-url-can-use-xdg-open. + (or (getenv "DISPLAY") (getenv "WAYLAND_DISPLAY")) + (executable-find "xdg-email"))) (defun report-emacs-bug-insert-to-mailer () "Send the message to your preferred mail client. diff --git a/lisp/net/browse-url.el b/lisp/net/browse-url.el index 04b49c4..20ae072 100644 --- a/lisp/net/browse-url.el +++ b/lisp/net/browse-url.el @@ -45,7 +45,7 @@ ;; browse-url-generic arbitrary ;; browse-url-default-windows-browser MS-Windows browser ;; browse-url-default-macosx-browser macOS browser -;; browse-url-xdg-open Free Desktop xdg-open on Gnome, KDE, Xfce4, LXDE +;; browse-url-xdg-open freedesktop.org xdg-open ;; browse-url-kde KDE konqueror (kfm) ;; browse-url-elinks Elinks Don't know (tried with 0.12.GIT) @@ -944,36 +944,14 @@ browse-url-default-browser (defun browse-url-can-use-xdg-open () "Return non-nil if the \"xdg-open\" program can be used. -xdg-open is a desktop utility that calls your preferred web browser. -This requires you to be running either Gnome, KDE, Xfce4 or LXDE." - (and (getenv "DISPLAY") - (executable-find "xdg-open") - ;; xdg-open may call gnome-open and that does not wait for its child - ;; to finish. This child may then be killed when the parent dies. - ;; Use nohup to work around. See bug#7166, bug#8917, bug#9779 and - ;; http://lists.gnu.org/archive/html/emacs-devel/2009-07/msg00279.html - (executable-find "nohup") - (or (getenv "GNOME_DESKTOP_SESSION_ID") - ;; GNOME_DESKTOP_SESSION_ID is deprecated, check on Dbus also. - (condition-case nil - (eq 0 (call-process - "dbus-send" nil nil nil - "--dest=org.gnome.SessionManager" - "--print-reply" - "/org/gnome/SessionManager" - "org.gnome.SessionManager.CanShutdown")) - (error nil)) - (equal (getenv "KDE_FULL_SESSION") "true") - (condition-case nil - (eq 0 (call-process - "/bin/sh" nil nil nil - "-c" - ;; FIXME use string-match rather than grep. - "xprop -root _DT_SAVE_MODE|grep xfce4")) - (error nil)) - (member (getenv "DESKTOP_SESSION") '("LXDE" "Lubuntu")) - (equal (getenv "XDG_CURRENT_DESKTOP") "LXDE")))) - +xdg-open is a desktop utility that calls your preferred web browser." + ;; The exact set of situations where xdg-open works is complicated, + ;; and it would be a pain to duplicate xdg-open's situation-specific + ;; code here, as the code is a moving target. So assume that + ;; xdg-open will work if there is a graphical display; this should + ;; be good enough for platforms Emacs is likely to be running on. + (and (or (getenv "DISPLAY") (getenv "WAYLAND_DISPLAY")) + (executable-find "xdg-open"))) ;;;###autoload (defun browse-url-xdg-open (url &optional ignored) -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-03-10 3:13 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-02-17 18:16 bug#25778: 25.1; [PATCH] Drastically simplify xdg-open check Vasilij Schneidermann 2017-02-17 18:38 ` Eli Zaretskii 2017-02-17 18:55 ` Vasilij Schneidermann 2017-02-18 3:39 ` Glenn Morris 2017-02-18 3:46 ` Glenn Morris 2017-02-18 8:55 ` Vasilij Schneidermann 2017-02-19 0:31 ` Glenn Morris 2017-02-19 13:06 ` Vasilij Schneidermann 2017-02-18 7:58 ` Eli Zaretskii 2017-02-18 8:50 ` Vasilij Schneidermann 2017-03-10 3:13 ` Paul Eggert
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.