Subject: Re: bug#71450: [PATCH] Wrong eww-history-position after desktop restore if within history

From: James Thomas <[email-masked]>
Date: Tue, 11 Jun 2024 03:06:31 +0530

Jim Porter wrote:

> On 6/9/2024 5:23 AM, James Thomas via Bug reports for GNU Emacs, the
> Swiss army knife of text editors wrote:
>> This is a patch that I think is simple enough to forgo the extensive
>> testing which it hasn't been subjected to.
>
> Thanks for the patch.
>
>> +          :history-position
>> +          (cl-position
>> +           (elt history eww-history-position)
>> +           rval :test #'eww-desktop-history-duplicate))))
>
> Two questions here:
>
> 1. Is that the right test function? I'd have expected 'eq', since we
> want to find the position where our history index has moved to, right?
>
> 2. Should this part check for 'eww-desktop-remove-duplicates' too? If
> that option is nil, I think we could avoid the 'cl-position' call. Or
> maybe lift the 'eww-desktop-remove-duplicates' call outside of the
> 'list' and just construct two totally different lists in the THEN/ELSE
> blocks.

Here's an updated patch, which I've tested somewhat:

From bc0e3f2653e1c4c1f683d4b10ff139dbf963ee8d Mon Sep 17 00:00:00 2001
From: James Thomas <[email-masked]>
Date: Tue, 11 Jun 2024 03:00:33 +0530
Subject: [PATCH] Account for duplicate removal on restoring
 eww-history-position.

* lisp/net/eww.el (eww-desktop-misc-data): Add :history-position
(eww-restore-desktop): Use it.
(desktop-locals-to-save): Remove the raw variable.
---
 lisp/net/eww.el | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index 977210e9cc8..fd8f80065b1 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -2754,11 +2754,20 @@ eww-desktop-misc-data
 Generally, the list should not include the (usually overly large)
 :dom, :source and :text properties."
   (let ((history (mapcar #'eww-desktop-data-1
-                         (cons eww-data eww-history))))
-    (list :history (if eww-desktop-remove-duplicates
-                       (cl-remove-duplicates
-                        history :test #'eww-desktop-history-duplicate)
-                     history))))
+                         (cons eww-data eww-history)))
+        (posn eww-history-position) rval)
+    (list :history
+          (if eww-desktop-remove-duplicates
+              (prog1
+                  (setq
+                   rval (cl-remove-duplicates
+                         history :test #'eww-desktop-history-duplicate))
+                (setq posn
+                      (cl-position
+                       (elt history eww-history-position)
+                       rval :test #'eq)))
+            history)
+          :history-position posn)))

 (defun eww-restore-desktop (file-name buffer-name misc-data)
   "Restore an eww buffer from its desktop file record.
@@ -2772,7 +2781,8 @@ eww-restore-desktop
     (setq eww-history       (cdr (plist-get misc-data :history))
 	  eww-data      (or (car (plist-get misc-data :history))
 			    ;; backwards compatibility
-			    (list :url (plist-get misc-data :uri))))
+			    (list :url (plist-get misc-data :uri)))
+          eww-history-position (plist-get misc-data :history-position))
     (unless file-name
       (when (plist-get eww-data :url)
 	(cl-case eww-restore-desktop
@@ -2784,8 +2794,6 @@ eww-restore-desktop
     ;; .
     (current-buffer)))

-(add-to-list 'desktop-locals-to-save
-	     'eww-history-position)
 (add-to-list 'desktop-buffer-mode-handlers
              '(eww-mode . eww-restore-desktop))

--
2.40.1

Regards,
James