Subject: bug#75354: (29.4; eww buffer is not displayed correctly when used from bookmark-jump )

From: Ship Mints <[email-masked]>
Date: Mon, 17 Mar 2025 06:41:32 -0400

On Mon, Mar 17, 2025 at 6:35 AM Ship Mints <[email-masked]> wrote:

> On Sun, Mar 16, 2025 at 5:10 PM Ship Mints <[email-masked]> wrote:
>
>> On Sat, Mar 15, 2025 at 10:18 AM Ship Mints <[email-masked]> wrote:
>>
>>> On Sat, Mar 15, 2025 at 1:37 AM Thierry Volpiatto <[email-masked]>
>>> wrote:
>>>
>>>> Ship Mints <[email-masked]> writes:
>>>>
>>>> > I have workarounds that work only for the most simplistic cases.  Many
>>>> > of our bookmarks themselves contain embedded bookmarks and bookmark
>>>> > references (which are individually addressable so can be used
>>>> > separately) with window-states we need to restore in tab-bar tabs that
>>>> > they represent.
>>>>
>>>> I don't really understand what your packages are doing or are intended
>>>> doing, but FWICS in bufferlo: You are using in some places
>>>> (bookmark-jump name #'ignore); why don't you do all this work (restore
>>>> window-states in tab) in DISPLAY-FUNCTION instead of using `ignore`?
>>>> Your handler would be much simpler by moving the window-state-put and
>>>> alike calls in DISPLAY-FUNCTION:
>>>>
>>>> (bookmark-jump name #'your_function_restoring_window_or_frame_state)
>>>>
>>>> Using (bookmark-jump name #'ignore) with all the code that jump to
>>>> frame/tab etc... in the handler is just a workaround to fix the previous
>>>> buggy behavior of bookmark--jump-via. IMO.
>>>>
>>>> It would be good to start with a good example or recipe to see if we can
>>>> find a good solution.
>>>>
>>>
>>> We need the bookmarks to work from the bookmark menu where no
>>> display-function overrides are supported.
>>>
>>> I suggest we add bookmark-record keys that indicate to bookmark-jump to
>>> inhibit/or allow messing with window-configurations.  The bufferlo
>>> bookmarks (and Adam's if he wants) would contain these hint keys.
>>>
>>> '(bookmark-jump-inhibit-window-actions . t) ; or whatever we come up with
>>>
>>> I can contrive an example, if necessary, but I believe y'all get the
>>> point.  Nested bookmarks whose handlers expect their window-configurations
>>> not to be messed with up the chain, will always be broken without
>>> additional controls.
>>>
>>
>> The attached patch implements such a scheme that works for us, and is
>> transparent to other bookmark uses.
>>
>
> Perhaps we should restore bookmark--jump-via to its previous behavior and
> better document the "rules of the road" for bookmark handlers.  For simple
> file- and point-based bookmarks, handlers need to ensure that when they
> return, the selected window and current buffer are what's intended.  For
> bookmark handlers that perform other actions, those rules need not apply to
> leverage the bookmark infrastructure.
>
> eww's handler could simply do this itself since it seems eww's url opening
> behavior warrants this:
>
> (defun eww-bookmark-jump (bookmark)
>   "Default bookmark handler for EWW buffers."
>   (save-window-excursion
>     (eww (bookmark-prop-get bookmark 'location))))
>
> I'd also suggest that we recommend adding an additional property to a
> bookmark-handler function that could inhibit bookmark--jump-via's call to
> display-func entirely.  In our case, when called programmatically, we use
> #'ignore, but when the bookmark menu is used, we'd prefer to skip the
> default pop-to-buffer-same-window.
>

Here's an example of another handler in the wild that handles its own
save-window-excursion:

https://github.com/emacsmirror/info-plus/blob/c6e26367abd2e99791f6e85fced2383de9ec605a/info%2B.el#L7025C1-L7039C98

(defun Info-bookmark-jump (bookmark)
  "Handler function for record returned by `Info-bookmark-make-record'.
BOOKMARK is a bookmark name or a bookmark record.

If `Info-bookmark-use-only-node-not-file-flag' is nil, and the
recorded Info file is readable, then use it.  If not, then go to the
recorded Info node in the manual for the current Emacs version."
  (let* ((absfile    (bookmark-prop-get bookmark 'filename))
         (file       (if (or Info-bookmark-use-only-node-not-file-flag
 (not (file-readable-p absfile)))
                         (file-name-nondirectory absfile)
                       absfile))
         (info-node  (bookmark-prop-get bookmark 'info-node))
         (buf        (save-window-excursion ; Vanilla FIXME: doesn't work
with frames!
                       (Info-find-node file info-node) (current-buffer))))
    (bookmark-default-handler `("" (buffer . ,buf) .
,(bookmark-get-bookmark-record bookmark)))))