GNU bug report logs - #25091
26.0.50; shr-map hides gnus-article keys

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 25091 in the body.
You can then email your comments to 25091 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Katsumi Yamaoka <yamaoka <at> jpl.org>
To: bug-gnu-emacs <at> gnu.org
Cc: 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
Subject: 26.0.50; shr-map hides gnus-article keys
Date: Fri, 02 Dec 2016 18:08:39 +0900

[Message part 1 (text/plain, inline)]

Hi,

Jidanni mailed me that TAB doesn't move point to attachments
existing in the bottom of an html article of Gnus like this:

,---- html part
|......
|       ...link...
|               ......
|                       ...link...
|       ......
`----
[2. application/pdf; foo.pdf]...
[3. application/octet-stream; bar.txt]...

An example is attached.

Each link has `shr-map' that overrides `gnus-article-mode-map'
of which the parent is `widget-keymap'.  TAB on a link invokes
`shr-next-link', not `widget-forward', so performing it on the
last link doesn't move point to the attachment, whereas TAB on
the html part other than links, i.e., `widget-forward', moves
point to the next link.  I think the behavior should be the same
as that of non-html articles.

I'm not quite sure if it is the right way, but tried fixing it
as follows:

[Message part 2 (text/x-patch, inline)]

--- shr.el~	2016-11-29 10:20:10.401598400 +0000
+++ shr.el	2016-12-02 09:04:23.642006900 +0000
@@ -344,8 +344,13 @@
      ((or (eobp)
           (not (setq skip (text-property-not-all (point) (point-max)
                                                  'shr-url nil))))
-      (goto-char start)
-      (message "No next link"))
+      (let ((command (lookup-key (current-local-map) (this-command-keys))))
+	(unless (and command
+		     (condition-case nil
+			 (progn (call-interactively command) t)
+		       (error nil)))
+	  (goto-char start)
+	  (message "No next link"))))
      (t
       (goto-char skip)
       (message "%s" (get-text-property (point) 'help-echo))))))
@@ -364,9 +369,13 @@
 		(not (setq found (get-text-property (point) 'help-echo))))
       (forward-char -1))
     (if (not found)
-	(progn
-	  (message "No previous link")
-	  (goto-char start))
+	(let ((command (lookup-key (current-local-map) (this-command-keys))))
+	  (unless (and command
+		       (condition-case nil
+			   (progn (call-interactively command) t)
+			 (error nil)))
+	    (message "No previous link")
+	    (goto-char start)))
       ;; Put point at the start of the link.
       (while (and (not (bobp))
 		  (get-text-property (point) 'help-echo))

[html+attachments.gz (application/x-gunzip, attachment)]

Message #8 received at 25091 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Katsumi Yamaoka <yamaoka <at> jpl.org>
Cc: 25091 <at> debbugs.gnu.org,
 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
Subject: Re: bug#25091: 26.0.50; shr-map hides gnus-article keys
Date: Fri, 02 Dec 2016 21:26:20 -0500

Katsumi Yamaoka <yamaoka <at> jpl.org> writes:

>
> I'm not quite sure if it is the right way, but tried fixing it
> as follows:

[...]

> +      (let ((command (lookup-key (current-local-map) (this-command-keys))))
> +	(unless (and command
> +		     (condition-case nil
> +			 (progn (call-interactively command) t)
> +		       (error nil)))
> +	  (goto-char start)
> +	  (message "No next link"))))

There's a similar attempt in yasnippet to try use the original binding.
It's generally been troublesome.  I'm planning to switch it to use a
conditional binding as described at [1].

(define-key <map> <key>
  `(menu-item "" <my-cmd> :filter ,(lambda (cmd) (if <my-predicate> cmd))))

[1]: http://stackoverflow.com/questions/16090517/elisp-conditionally-change-keybinding/22863701#22863701

Message #11 received at 25091 <at> debbugs.gnu.org (full text, mbox):

From: Katsumi Yamaoka <yamaoka <at> jpl.org>
To: npostavs <at> users.sourceforge.net
Cc: 25091 <at> debbugs.gnu.org,
 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
Subject: Re: bug#25091: 26.0.50; shr-map hides gnus-article keys
Date: Mon, 05 Dec 2016 15:57:12 +0900

[Message part 1 (text/plain, inline)]

On Fri, 02 Dec 2016 21:26:20 -0500, npostavs <at> users.sourceforge.net wrote:
> There's a similar attempt in yasnippet to try use the original binding.
> It's generally been troublesome.

I realized it is not a coincidence that two TAB commands, one is
provided by `gnus-article-mode-map' and the other is provided by
`shr-map', behave similarly in the Gnus article buffer.  Why TAB
-- `widget-forward' -- moves the point to a shr link in addition
to attachment buttons is because Gnus adds widget buttons to shr
links purposely using `mm-convert-shr-links' (see mm-decode.el).
So, I believe it is natural to make `shr-next-link' behave like
`widget-forward' as well.  Furthermore, what should make it do so
should be `mm-convert-shr-links', not shr functions (as a patch
I posted first).  In other words, the command that TAB invokes
on a widget button added to a shr link should be `widget-forward',
not `shr-next-link'.

,---- background
| Normally TAB in the Gnus article buffer runs `widget-forward'
| that moves point to the next attachment button or a shr link.
| However, TAB on a shr link in the Gnus article buffer runs
| `shr-next-link' that moves point to only the next shr link.
| So is `shr-previous-link'.
`----

A new patch is below.

> I'm planning to switch it to use a conditional binding as described at [1].

> (define-key <map> <key>
>   `(menu-item "" <my-cmd> :filter ,(lambda (cmd) (if <my-predicate> cmd))))

> [1]: http://stackoverflow.com/questions/16090517/elisp-conditionally-change-keybinding/22863701#22863701

Thanks.  But this case seems to not necessarily be related.

[Message part 2 (text/x-patch, inline)]

--- mm-decode.el~	2016-07-25 23:45:06.237009600 +0000
+++ mm-decode.el	2016-12-05 06:55:13.007620000 +0000
@@ -1859,6 +1859,10 @@
 	(dolist (key (where-is-internal #'widget-button-click widget-keymap))
 	  (unless (lookup-key keymap key)
 	    (define-key keymap key #'ignore)))
+	;; Avoid `shr-next-link' and `shr-previous-link' so as to run
+	;; `widget-forward' and `widget-backward' instead.
+	(substitute-key-definition 'shr-next-link nil keymap)
+	(substitute-key-definition 'shr-previous-link nil keymap)
 	(dolist (overlay (overlays-at start))
 	  (overlay-put overlay 'face nil))
 	(setq start end)))))

Message #14 received at 25091 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Katsumi Yamaoka <yamaoka <at> jpl.org>
Cc: 25091 <at> debbugs.gnu.org,
 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
Subject: Re: bug#25091: 26.0.50; shr-map hides gnus-article keys
Date: Mon, 05 Dec 2016 09:06:14 -0500

tags 25091 patch
quit

Katsumi Yamaoka <yamaoka <at> jpl.org> writes:
>
> A new patch is below.
>

Looks good to me.  I couldn't figure out how to use your attached
example (in the first message) to test it though.

Message #19 received at 25091 <at> debbugs.gnu.org (full text, mbox):

From: Katsumi Yamaoka <yamaoka <at> jpl.org>
To: npostavs <at> users.sourceforge.net
Cc: 25091 <at> debbugs.gnu.org,
 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
Subject: Re: bug#25091: 26.0.50; shr-map hides gnus-article keys
Date: Tue, 06 Dec 2016 07:59:01 +0900

On Mon, 05 Dec 2016 09:06:14 -0500, npostavs <at> users.sourceforge.net wrote:
> Looks good to me.  I couldn't figure out how to use your attached
> example (in the first message) to test it though.

Thanks.  I'll install it later.  The example is just a raw email.
You can read it using Gnus as follows:

・Save the attachment to a file; you don't have to gunzip it.
・Go to the Gnus group buffer.
・Open the file using the `G f' command as a new nndoc group.
・Type `M-g' on the group line to update it.
・Enter the group.
・The article #1 is it!
・Try TAB keys in the article buffer.
・You can copy the article to the other group if needed.
・Quit the group; type `C-k' on the group line to kill it.

Regards,

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.