bug#45623: 28.0.50; widget-backward sometimes skips one widget

[Mauro Aranda]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> I don't understand why that call to backward-char is there.  Does anyone
>> know of a situation where it is useful?
>
> If point is just after a button, it makes C-M-i not going to that
> button, but the previous one?  But that doesn't really sound like
> something you'd want, either?
>
> So, no, I don't see any situation where it would useful -- looks like a
> misfeature to me.

Agreed.  So, any objections to installing this patch and see if
something breaks?

>From 86c211725ab39ddfc569a16fefd0db6724651b43 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Sun, 3 Jan 2021 11:27:02 -0300
Subject: [PATCH] Don't skip widgets when moving backward

* lisp/wid-edit.el (widget-move): Remove code that caused
widget-backward to skip a previous adjacent widget when moving
backward from the start of a widget.  (Bug#45623)

* test/lisp/wid-edit-tests.el (widget-test-widget-backward): New test.
---
 lisp/wid-edit.el            |  1 -
 test/lisp/wid-edit-tests.el | 22 ++++++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index f920130226..8b10d71dcb 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -1204,7 +1204,6 @@ widget-move
 ARG may be negative to move backward.
 When the second optional argument is non-nil,
 nothing is shown in the echo area."
-  (or (bobp) (> arg 0) (backward-char))
   (let ((wrapped 0)
        (number arg)
        (old (widget-tabable-at)))
diff --git a/test/lisp/wid-edit-tests.el b/test/lisp/wid-edit-tests.el
index 35235c6566..f2e8c321f5 100644
--- a/test/lisp/wid-edit-tests.el
+++ b/test/lisp/wid-edit-tests.el
@@ -301,4 +301,26 @@ widget-test-option-can-handle-inlinable-choice
       (should child)
       (should (equal (widget-value widget) '((1 "One")))))))
 
+(ert-deftest widget-test-widget-backward ()
+  "Test that `widget-backward' works OK."
+  (with-temp-buffer
+    (widget-insert "Testing.\n\n")
+    (dolist (el '("First" "Second" "Third"))
+      (widget-create 'push-button el))
+    (widget-insert "\n")
+    (goto-char (point-min))
+    (use-local-map widget-keymap)
+    (widget-setup)
+    ;; Check that moving from the widget's start works.
+    (widget-forward 3)
+    (should (string= "Third" (widget-value (widget-at))))
+    (widget-backward 1)
+    (should (string= "Second" (widget-value (widget-at))))
+    ;; Check that moving from inside the widget works.
+    (goto-char (point-min))
+    (widget-forward 3)
+    (forward-char)
+    (widget-backward 1)
+    (should (string= "Second" (widget-value (widget-at))))))
+
 ;;; wid-edit-tests.el ends here
-- 
2.29.2