bug#52856: 29.0.50; Problematic handling of webkit xwidget bookmarks

[Po Lu via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

Stephen Berman <stephen.berman@gmx.net> writes:

> How's this: "Jump to the web page bookmarked by the bookmark record
> BOOKMARK."?

Sure.

> Thanks.  Here's the latest version:

Thanks, LGTM.  Have you signed the copyright papers yet?

> 2021-12-30  Stephen Berman  <stephen.berman@gmx.net>
>
> 	Fix handling of webkit xwidget bookmarks
>
> 	Make jumping to a bookmarked webkit xwidget in another window or
> 	another frame show the xwidget only in that window or frame and
> 	refactor new session code used by the bookmark code (bug#52856).
> 	In addition, make xwidget-webkit-clone-* commands work.
>
> 	* lisp/xwidget.el (xwidget-webkit-clone-and-split-below)
> 	(xwidget-webkit-clone-and-split-right): Unbreak these functions by
> 	passing just the URL to them, not the message displaying it.
> 	(xwidget-webkit-bookmark-jump-new-session): Adjust doc string,
> 	rephrasing and removing a customization suggestion that cannot
> 	take effect.
> 	(xwidget-webkit-bookmark-jump-handler): New autoloaded function.
> 	(xwidget-webkit-bookmark-make-record): Use it.
> 	(xwidget-webkit--create-new-session-buffer): New function
> 	extracted from `xwidget-webkit-new-session'.
> 	(xwidget-webkit-new-session): Use it.
>
> diff --git a/lisp/xwidget.el b/lisp/xwidget.el
> index ce9839ebd3..1f92966492 100644
> --- a/lisp/xwidget.el
> +++ b/lisp/xwidget.el
> @@ -138,7 +138,7 @@ xwidget-webkit-clone-and-split-below
>  Get the URL of current session, then browse to the URL
>  in `split-window-below' with a new xwidget webkit session."
>    (interactive nil xwidget-webkit-mode)
> -  (let ((url (xwidget-webkit-current-url)))
> +  (let ((url (xwidget-webkit-uri (xwidget-webkit-current-session))))
>      (with-selected-window (split-window-below)
>        (xwidget-webkit-new-session url))))
>
> @@ -147,7 +147,7 @@ xwidget-webkit-clone-and-split-right
>  Get the URL of current session, then browse to the URL
>  in `split-window-right' with a new xwidget webkit session."
>    (interactive nil xwidget-webkit-mode)
> -  (let ((url (xwidget-webkit-current-url)))
> +  (let ((url (xwidget-webkit-uri (xwidget-webkit-current-session))))
>      (with-selected-window (split-window-right)
>        (xwidget-webkit-new-session url))))
>
> @@ -531,24 +531,31 @@ xwidget-webkit-save-as-file
>  ;;; Bookmarks integration
>
>  (defcustom xwidget-webkit-bookmark-jump-new-session nil
> -  "Control bookmark jump to use new session or not.
> -If non-nil, use a new xwidget webkit session after bookmark jump.
> -Otherwise, it will use `xwidget-webkit-last-session'.
> -When you set this variable to nil, consider further customization with
> -`xwidget-webkit-last-session-buffer'."
> +  "Whether to jump to a bookmarked URL in a new xwidget webkit session.
> +If non-nil, create a new xwidget webkit session, otherwise use
> +the value of `xwidget-webkit-last-session'."
>    :version "28.1"
>    :type 'boolean)
>
>  (defun xwidget-webkit-bookmark-make-record ()
> -  "Create bookmark record in webkit xwidget.
> -See `xwidget-webkit-bookmark-jump-new-session' for whether this
> -should create a new session or not."
> +  "Create a bookmark record for a webkit xwidget."
>    (nconc (bookmark-make-record-default t t)
>           `((page . ,(xwidget-webkit-uri (xwidget-webkit-current-session)))
> -           (handler  . (lambda (bmk)
> -                         (xwidget-webkit-browse-url
> -                          (bookmark-prop-get bmk 'page)
> -                          xwidget-webkit-bookmark-jump-new-session))))))
> +           (handler . xwidget-webkit-bookmark-jump-handler))))
> +
> +;;;###autoload
> +(defun xwidget-webkit-bookmark-jump-handler (bookmark)
> +  "Jump to the web page bookmarked by the bookmark record BOOKMARK.
> +If `xwidget-webkit-bookmark-jump-new-session' is non-nil, create
> +a new xwidget-webkit session, otherwise use an existing session."
> +  (let* ((url (bookmark-prop-get bookmark 'page))
> +	 (xwbuf (if (or xwidget-webkit-bookmark-jump-new-session
> +                        (not (xwidget-webkit-current-session)))
> +	            (xwidget-webkit--create-new-session-buffer url)
> +                  (xwidget-buffer (xwidget-webkit-current-session)))))
> +    (with-current-buffer xwbuf
> +      (xwidget-webkit-goto-uri (xwidget-webkit-current-session) url))
> +    (set-buffer xwbuf)))
>
>  ;;; xwidget webkit session
>
> @@ -796,37 +803,44 @@ xwidget-webkit-adjust-size-in-frame
>    (add-to-list 'window-size-change-functions
>                 'xwidget-webkit-adjust-size-in-frame))
>
> -(defun xwidget-webkit-new-session (url &optional callback)
> -  "Create a new webkit session buffer with URL."
> +(defun xwidget-webkit--create-new-session-buffer (url &optional callback)
> +  "Create a new webkit session buffer to display URL in an xwidget.
> +Optional function CALLBACK specifies the callback for webkit xwidgets;
> +see `xwidget-webkit-callback'."
>    (let* ((bufname
> -          ;; Generate a temp-name based on current buffer name. it
> -          ;; will be renamed by `xwidget-webkit-callback' in the
> -          ;; future. This approach can limit flicker of buffer-name in
> -          ;; mode-line.
> +          ;; Generate a temp-name based on current buffer name.  The
> +          ;; buffer will subsequently be renamed by
> +          ;; `xwidget-webkit-callback'.  This approach can avoid
> +          ;; flicker of buffer-name in mode-line.
>            (generate-new-buffer-name (buffer-name)))
>           (callback (or callback #'xwidget-webkit-callback))
>           (current-session (xwidget-webkit-current-session))
>           xw)
> -    (setq xwidget-webkit-last-session-buffer (switch-to-buffer
> -                                              (get-buffer-create bufname)))
> +    (setq xwidget-webkit-last-session-buffer (get-buffer-create bufname))
>      ;; The xwidget id is stored in a text property, so we need to have
>      ;; at least character in this buffer.
>      ;; Insert invisible url, good default for next `g' to browse url.
> -    (let ((start (point)))
> -      (insert url)
> -      (put-text-property start (+ start (length url)) 'invisible t)
> -      (setq xw (xwidget-insert
> -                start 'webkit bufname
> -                (xwidget-window-inside-pixel-width (selected-window))
> -                (xwidget-window-inside-pixel-height (selected-window))
> -                nil current-session)))
> -    (when xwidget-webkit-cookie-file
> -      (xwidget-webkit-set-cookie-storage-file
> -       xw (expand-file-name xwidget-webkit-cookie-file)))
> -    (xwidget-put xw 'callback callback)
> -    (xwidget-put xw 'display-callback #'xwidget-webkit-display-callback)
> -    (xwidget-webkit-mode)
> -    (xwidget-webkit-goto-uri (xwidget-webkit-last-session) url)))
> +    (with-current-buffer xwidget-webkit-last-session-buffer
> +      (let ((start (point)))
> +        (insert url)
> +        (put-text-property start (+ start (length url)) 'invisible t)
> +        (setq xw (xwidget-insert
> +                  start 'webkit bufname
> +                  (xwidget-window-inside-pixel-width (selected-window))
> +                  (xwidget-window-inside-pixel-height (selected-window))
> +                  nil current-session)))
> +      (when xwidget-webkit-cookie-file
> +        (xwidget-webkit-set-cookie-storage-file
> +         xw (expand-file-name xwidget-webkit-cookie-file)))
> +      (xwidget-put xw 'callback callback)
> +      (xwidget-put xw 'display-callback #'xwidget-webkit-display-callback)
> +      (xwidget-webkit-mode))
> +    xwidget-webkit-last-session-buffer))
> +
> +(defun xwidget-webkit-new-session (url)
> +  "Display URL in a new webkit xwidget."
> +  (switch-to-buffer (xwidget-webkit--create-new-session-buffer url))
> +  (xwidget-webkit-goto-uri (xwidget-webkit-last-session) url))
>
>  (defun xwidget-webkit-import-widget (xwidget)
>    "Create a new webkit session buffer from XWIDGET, an existing xwidget.