Subject: Re: bug#74730: [PATCH] 30.0.92; eww-browse-with-external-browser and eww-follow-link should use browse-url-with-browser-kind

From: Daniel Mendler <[email-masked]>
Date: Sun, 08 Dec 2024 14:00:56 +0100

Eli Zaretskii <[email-masked]> writes:

>> > If we can make it so that existing customizations of
>> > browse-url-secondary-browser-function will keep working as they did,
>> > then the backward incompatibility issue will disappear, and such a
>> > change becomes possible.  But in any case, the doc string of
>> > browse-url-secondary-browser-function should be amended if we are
>> > going to support its setting to eww.
>> 
>> Okay, I can do that.
>> 
>> > Also, are we sure all the relevant functions always have the
>> > 'browse-url-browser-kind property? what about user-defined functions,
>> > for example?
>> 
>> User-defined functions may not have the property. But we can be
>> conservative and preserve the existing behavior in the case where the
>> property is unavailable, treating the missing property like the value
>> `external'. Only if the property is found and `internal', the
>> `browse-url-with-browser-kind' will be used to make sure that an
>> external browser is used. As I mentioned, alternatively one can be even
>> more conservative and only use `browse-url-with-browser-kind' if
>> `browse-url-secondary-browser-function' is set to `eww-browse-url'. That
>> might be a little bit too restrictive, but it would be completely
>> backward compatible.
>
> Looking forward to seeing an updated patch which preserves the current
> behavior wrt browse-url-secondary-browser-function's customization.
>
> Thanks.

Updated patch attached.

>From 9d782ed2123b34fd09268f6c83305d7d9d4acf7d Mon Sep 17 00:00:00 2001
From: Daniel Mendler <[email-masked]>
Date: Sat, 7 Dec 2024 22:56:15 +0100
Subject: [PATCH] eww: Improve eww-browse-with-external-browser

Guarantee that an external browser is used by EWW if
`browse-url-secondary-browser-function' is set to
`eww-browse-url'.

* lisp/net/eww.el (eww-browse-with-external-browser): Use
`browse-url-secondary-browser-function' if it does not
correspond to EWW, otherwise fall back to `browse-url'.
(eww-follow-link): Use `eww-browse-with-external-browser' if the
EXTERNAL prefix argument is non-nil.
* lisp/net/browse-url.el
(browse-url-secondary-browser-function): Update docstring.
---
 lisp/net/browse-url.el |  8 ++++----
 lisp/net/eww.el        | 14 ++++++++------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/lisp/net/browse-url.el b/lisp/net/browse-url.el
index c10bc671a88..c9d8b2ee80d 100644
--- a/lisp/net/browse-url.el
+++ b/lisp/net/browse-url.el
@@ -198,10 +198,10 @@ browse-url-browser-function
 
 (defcustom browse-url-secondary-browser-function 'browse-url-default-browser
   "Function used to launch an alternative browser.
-This is usually an external browser (that is, not eww or w3m),
-used as the secondary browser choice, typically when a prefix
-argument is given to a URL-opening command in those modes that
-support this (for instance, eww/shr).
+
+This is browser is used as the secondary browser choice, typically
+when a prefix argument is given to a URL-opening command in those
+modes that support this (for instance `goto-addr-at-point', eww or shr).
 
 Also see `browse-url-browser-function'."
   :version "27.1"
diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index 4d4d4d6beac..b2d295a4dcf 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -2142,10 +2142,13 @@ eww-submit
 
 (defun eww-browse-with-external-browser (&optional url)
   "Browse the current URL with an external browser.
-The browser to used is specified by the
-`browse-url-secondary-browser-function' variable."
+Calls `browse-url-secondary-browser-function' if it does not correspond
+to EWW.  Otherwise `browse-url' is used."
   (interactive nil eww-mode)
-  (funcall browse-url-secondary-browser-function
+  (funcall (if (memq browse-url-secondary-browser-function
+                     '(eww eww-browse-url))
+               #'browse-url
+             browse-url-secondary-browser-function)
            (or url (plist-get eww-data :url))))
 
 (defun eww-remove-tracking (url)
@@ -2162,8 +2165,7 @@ eww--transform-url
 (defun eww-follow-link (&optional external mouse-event)
   "Browse the URL under point.
 If EXTERNAL is single prefix, browse the URL using
-`browse-url-secondary-browser-function'.
-
+`eww-browse-with-external-browser'.
 If EXTERNAL is double prefix, browse in new buffer."
   (interactive
    (list current-prefix-arg last-nonmenu-event)
@@ -2180,7 +2182,7 @@ eww-follow-link
       ;; and `browse-url-mailto-function'.
       (browse-url url))
      ((and (consp external) (<= (car external) 4))
-      (funcall browse-url-secondary-browser-function url)
+      (eww-browse-with-external-browser url)
       (shr--blink-link))
      ;; This is a #target url in the same page as the current one.
      ((and (setq target (url-target (url-generic-parse-url url)))
-- 
2.45.2