bug#25091: 26.0.50; shr-map hides gnus-article keys

bug#25091: 26.0.50; shr-map hides gnus-article keys

[Katsumi Yamaoka]

[-- Attachment #1: Type: text/plain, Size: 836 bytes --]

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:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1248 bytes --]

--- 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))

[-- Attachment #3: html+attachments.gz --]
[-- Type: application/x-gunzip, Size: 76029 bytes --]

bug#25091: 26.0.50; shr-map hides gnus-article keys

[npostavs]

Katsumi Yamaoka <yamaoka@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

bug#25091: 26.0.50; shr-map hides gnus-article keys

[Katsumi Yamaoka]

[-- Attachment #1: Type: text/plain, Size: 1600 bytes --]

On Fri, 02 Dec 2016 21:26:20 -0500, npostavs@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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 612 bytes --]

--- 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)))))

bug#25091: 26.0.50; shr-map hides gnus-article keys

[npostavs]

tags 25091 patch
quit

Katsumi Yamaoka <yamaoka@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.

bug#25091: 26.0.50; shr-map hides gnus-article keys

[Katsumi Yamaoka]

On Mon, 05 Dec 2016 09:06:14 -0500, npostavs@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,