bug#67791: 30.0.50; `t' command on Gnus summary buffer sometimes causes an error

bug#67791: 30.0.50; `t' command on Gnus summary buffer sometimes causes an error

[Katsumi Yamaoka]

Because the `gnus-article-treat-fold-headers' function does not
work as expected.  `t' command, i.e. `gnus-summary-toggle-header'
is invoked on the Gnus summary buffer, that runs by default
`gnus-article-treat-fold-headers' by way of `gnus-treat-article'.
It works on the article buffer and folds headers with the help of
the pixel-fill functions.  One of them, `pixel-fill-region' uses
the built-in `window-text-pixel-size' function, that is required
to run on the selected window in which there is the text to
measure the pixel size (since `pixel-fill-region' passes nil to
`window-text-pixel-size' as the 1st argument `WINDOW').
,----
| (window-text-pixel-size &optional WINDOW FROM TO ...)
|
| Return the size of the text of WINDOW's buffer in pixels.
| WINDOW must be a live window and defaults to the selected one.
`----
Therefore, the window should be that of the article.  However,
in that situation the selected one is that of the summary, as
mentioned above.  Because of this, the `window-text-pixel-size'
sometimes returns a funny value and it causes the error like
this:

Debugger entered--Lisp error: (error "The indentation (640) is wider than th...
  signal(error ("The indentation (640) is wider than the fill width (623)"))...
  error("The indentation (%s) is wider than the fill width (%s)" 640 623)...
  (progn (error "The indentation (%s) is wider than the fill width (%s)" ind...
  (if (> indentation pixel-width) (progn (error "The indentation (%s) is wid...
  (let ((indentation (car (window-text-pixel-size nil (line-beginning-positi...
  (save-excursion (goto-char start) (let ((indentation (car (window-text-pix...
  pixel-fill-region(101 181 623)
 [...]
  command-execute(gnus-summary-toggle-header)

Another possible cause might be the recent change (33b6de7a)
made in `window-text-pixel-size', because such an error doesn't
look to cause in Emacs 29.1.90 (the function before the change
might possibly have worked on the current buffer, not the
selected window).

Here is a quick hack:

* lisp/gnus/gnus-art.el (gnus-article-treat-fold-headers): Make sure
the article window is selected while running pixel-fill-region.

--8<---------------cut here---------------start------------->8---
--- gnus-art.el~	2023-12-10 23:22:23.410103300 +0000
+++ gnus-art.el	2023-12-12 02:11:11.364620000 +0000
@@ -2240,7 +2240,9 @@
         (if (not (gnus--variable-pitch-p (get-text-property (point) 'face)))
 	    (mail-header-fold-field)
           (forward-char 1)
-          (pixel-fill-region (point) (point-max) (pixel-fill-width)))
+          (save-window-excursion
+            (set-window-buffer nil (current-buffer))
+            (pixel-fill-region (point) (point-max) (pixel-fill-width))))
 	(goto-char (point-max))))))
 
 (defun gnus-article-treat-suspicious-headers ()
--8<---------------cut here---------------end--------------->8---

In GNU Emacs 30.0.50 (build 1, x86_64-pc-cygwin, GTK+ Version
 3.22.28, cairo version 1.17.4) of 2023-12-12 built on localhost
Windowing system distributor 'The Cygwin/X Project', version 11.0.12101008
Configured using:
 'configure 'CFLAGS=-O0 -g3' --verbose
 --infodir=/usr/local/info/emacs --with-x-toolkit=gtk3
 --with-cairo-xcb --with-imagemagick
 --with-native-compilation=no'

bug#67791: 30.0.50; `t' command on Gnus summary buffer sometimes causes an error

[Eli Zaretskii]

> Date: Tue, 12 Dec 2023 11:15:54 +0900
> From: Katsumi Yamaoka <yamaoka@jpl.org>
> 
> Because the `gnus-article-treat-fold-headers' function does not
> work as expected.  `t' command, i.e. `gnus-summary-toggle-header'
> is invoked on the Gnus summary buffer, that runs by default
> `gnus-article-treat-fold-headers' by way of `gnus-treat-article'.
> It works on the article buffer and folds headers with the help of
> the pixel-fill functions.  One of them, `pixel-fill-region' uses
> the built-in `window-text-pixel-size' function, that is required
> to run on the selected window in which there is the text to
> measure the pixel size (since `pixel-fill-region' passes nil to
> `window-text-pixel-size' as the 1st argument `WINDOW').
> ,----
> | (window-text-pixel-size &optional WINDOW FROM TO ...)
> |
> | Return the size of the text of WINDOW's buffer in pixels.
> | WINDOW must be a live window and defaults to the selected one.
> `----
> Therefore, the window should be that of the article.  However,
> in that situation the selected one is that of the summary, as
> mentioned above.  Because of this, the `window-text-pixel-size'
> sometimes returns a funny value and it causes the error like
> this:
> 
> Debugger entered--Lisp error: (error "The indentation (640) is wider than th...
>   signal(error ("The indentation (640) is wider than the fill width (623)"))...
>   error("The indentation (%s) is wider than the fill width (%s)" 640 623)...
>   (progn (error "The indentation (%s) is wider than the fill width (%s)" ind...
>   (if (> indentation pixel-width) (progn (error "The indentation (%s) is wid...
>   (let ((indentation (car (window-text-pixel-size nil (line-beginning-positi...
>   (save-excursion (goto-char start) (let ((indentation (car (window-text-pix...
>   pixel-fill-region(101 181 623)
>  [...]
>   command-execute(gnus-summary-toggle-header)
> 
> Another possible cause might be the recent change (33b6de7a)
> made in `window-text-pixel-size', because such an error doesn't
> look to cause in Emacs 29.1.90 (the function before the change
> might possibly have worked on the current buffer, not the
> selected window).

Did you try using the newer buffer-text-pixel-size instead?

bug#67791: 30.0.50; `t' command on Gnus summary buffer sometimes causes an error

[Katsumi Yamaoka]

On Tue, 12 Dec 2023 13:51:30 +0200, Eli Zaretskii wrote:
> Did you try using the newer buffer-text-pixel-size instead?

Thanks, but I didn't know and not tried it.  Probably the buffer
needs to be narrowed to FROM and TO, that are the arguments
passed to `window-text-pixel-size'.

Un?fortunately the error comes not to happen on today's Emacs
built for the unknown reason.  The article window is always
selected when `pixel-fill-region' runs in spite of invoking the
`t' command on the summary window.  So, I'm closing this bug,
but I'll reopen it if the issue arises again.

Regards,

bug#67791: 30.0.50; `t' command on Gnus summary buffer sometimes causes an error

[Eli Zaretskii]

> Date: Wed, 13 Dec 2023 11:11:38 +0900
> From: Katsumi Yamaoka <yamaoka@jpl.org>
> Cc: 67791-done@debbugs.gnu.org
> 
> On Tue, 12 Dec 2023 13:51:30 +0200, Eli Zaretskii wrote:
> > Did you try using the newer buffer-text-pixel-size instead?
> 
> Thanks, but I didn't know and not tried it.  Probably the buffer
> needs to be narrowed to FROM and TO, that are the arguments
> passed to `window-text-pixel-size'.

I don't think you need to narrow the buffer, no.

> Un?fortunately the error comes not to happen on today's Emacs
> built for the unknown reason.  The article window is always
> selected when `pixel-fill-region' runs in spite of invoking the
> `t' command on the summary window.  So, I'm closing this bug,
> but I'll reopen it if the issue arises again.

OK, thanks.  If it does arise again, please try using
buffer-text-pixel-size in that place, I think it fits better what
(AFAIU) the code wants to accomplish there.

bug#67791: 30.0.50; `t' command on Gnus summary buffer sometimes causes an error

[Katsumi Yamaoka]

reopen 67791

The bugs is getting back again with the error such as the one I
reported in <b4mr0jsmahx.fsf@jpl.org>
(<https://lists.gnu.org/archive/html/bug-gnu-emacs/2023-12/msg00690.html>).

On Wed, 13 Dec 2023 13:55:32 +0200, Eli Zaretskii wrote:
>> On Tue, 12 Dec 2023 13:51:30 +0200, Eli Zaretskii wrote:
>>> Did you try using the newer buffer-text-pixel-size instead?

>> Thanks, but I didn't know and not tried it.  Probably the buffer
>> needs to be narrowed to FROM and TO, that are the arguments
>> passed to `window-text-pixel-size'.

> I don't think you need to narrow the buffer, no.

`pixel-fill-region' measures the pixel width of the first letter
of a message header using `window-text-pixel-size' as follows:

(window-text-pixel-size nil (line-beginning-position) (point))

Where the point is just after the first letter of a header.  So,
how do we use `buffer-text-pixel-size' so to do the same thing?
It's nice if it can be used even when the window in which a letter
to be measured exists is not the selected one.  But if it's not
possible, I think the best way whould be to use the patch I posted
first:

--- gnus-art.el~	2023-12-10 23:22:23.410103300 +0000
+++ gnus-art.el	2023-12-12 02:11:11.364620000 +0000
@@ -2240,7 +2240,9 @@
         (if (not (gnus--variable-pitch-p (get-text-property (point) 'face)))
 	    (mail-header-fold-field)
           (forward-char 1)
-          (pixel-fill-region (point) (point-max) (pixel-fill-width)))
+          (save-window-excursion
+            (set-window-buffer nil (current-buffer))
+            (pixel-fill-region (point) (point-max) (pixel-fill-width))))
 	(goto-char (point-max))))))
 
 (defun gnus-article-treat-suspicious-headers ()

This makes `window-text-pixel-size' sure to run on the article
window.

Regards,

bug#67791: 30.0.50; `t' command on Gnus summary buffer sometimes causes an error

[Eli Zaretskii]

> Date: Wed, 20 Dec 2023 09:34:00 +0900
> From: Katsumi Yamaoka <yamaoka@jpl.org>
> Cc: 67791@debbugs.gnu.org
> 
> `pixel-fill-region' measures the pixel width of the first letter
> of a message header using `window-text-pixel-size' as follows:
> 
> (window-text-pixel-size nil (line-beginning-position) (point))
> 
> Where the point is just after the first letter of a header.  So,
> how do we use `buffer-text-pixel-size' so to do the same thing?

If this is a general problem with pixel-fill-region, then shouldn't
the save-window-excursion thing be in pixel-fill-region instead?

IOW, in pixel-fill-region, if the selected window doesn't display the
current buffer, use save-window-excursion to make it so (to avoid thej
costly save-window-excursion when the current buffer is already in the
selected window).

Or what am I missing?

bug#67791: 30.0.50; `t' command on Gnus summary buffer sometimes causes an error

[Katsumi Yamaoka]

On Wed, 20 Dec 2023 14:53:02 +0200, Eli Zaretskii wrote:
> If this is a general problem with pixel-fill-region, then shouldn't
> the save-window-excursion thing be in pixel-fill-region instead?

Currently only `gnus-article-treat-fold-headers' uses `pixel-fill-region'
in Emacs,

> IOW, in pixel-fill-region, if the selected window doesn't display the
> current buffer, use save-window-excursion to make it so (to avoid thej
> costly save-window-excursion when the current buffer is already in the
> selected window).

but it's a good idea, I think.