mu4e-message-kill-buffer doesn't restore previous window configuration

[thierryvolpiatto]

Describe the bug

When killing composition buffer (C-c C-k) previous window configuration is not restored.

How to Reproduce

1. Open an Email from mu4e headers buffer.
   You have now two windows, the headers window and the view buffer where your mail is.
2. Hit "W" or "R" to reply, you have now the headers window (same as before) and in place of the view buffer the composition buffer.
3. Change your mind and hit C-c C-k.
   You have now only the headers window. This because mu4e-message-kill-buffer set the view buffer before killing the composition buffer and also it check for windows that are no more existing.

What is expected is to be back to 1) when hitting C-c C-k.

The following patch is fixing it:

diff --git a/mu4e/mu4e-compose.el b/mu4e/mu4e-compose.el
index 7ceb365b..836d2072 100644
--- a/mu4e/mu4e-compose.el
+++ b/mu4e/mu4e-compose.el
@@ -461,14 +461,14 @@ It attempts to restore some mu4e window layout after killing the
 compose-buffer."
   (interactive)
   (let ((win (selected-window))
-        (view (mu4e-get-view-buffer))
+        ;; `mu4e-get-view-buffer' makes the view buffer current.
+        (view (save-selected-window (mu4e-get-view-buffer)))
         (hdrs (mu4e-get-headers-buffer)))
     (message-kill-buffer)
-    (when (window-live-p win) (delete-window win))
     ;; try to go back to some mu window if its live; otherwise do nothing.
-    (if (and (buffer-live-p view) (window-live-p (get-buffer-window view)))
+    (if (buffer-live-p view)
         (switch-to-buffer view)
-      (when (and (buffer-live-p hdrs) (window-live-p (get-buffer-window hdrs)))
+      (when (buffer-live-p hdrs)
         (switch-to-buffer hdrs)))))
 
 ;;; Crypto

Environment

LinuxMint and Emacs-29.1

Checklist

• [X ] you are running either an 1.8.x/1.10.x release or master (otherwise please upgrade)
• [ X] you can reproduce the problem without 3rd party extensions (including Doom/Evil, various extensions etc.)
• [ X] you have read all of the above

Please make sure you all items in the checklist are set/met before filing the ticket. Thank you!

[thierryvolpiatto]

I guess there is a similar issue as well when sending mail (instead of C-c C-k), but this time I always end up with 3 windows:
The headers window, a window handling previous buffer used before opening Mu4e, and the view buffer.
Didn't find how to fix it yet.

[thierryvolpiatto]

I guess there is a similar issue as well when sending mail (instead of C-c C-k), but this time I always end up with 3 windows:
The headers window, a window handling previous buffer used before opening Mu4e, and the view buffer.

This happen after tag 1.11.24, I will try to bissect further to find the offending commit.

[thierryvolpiatto]

Culprit is commit 8abf098

[djcb]

Ah, thanks for checking.

There's a bit of messy massaging around the way message does its thing, and there is still some more to do...

[djcb]

Ah, hadn't even noticed the new behavior. Anyway, the proposed change seems to do the job, I'll push a fix soon. Thanks!

[thierryvolpiatto]

"Dirk-Jan C. Binnema" ***@***.***> writes:

Ah, thanks for checking. There's a bit of messy massaging around the way message does its thing, and there is still some more to do...

Perhaps we could open a new bug report about this? To resume, I have on my main frame two buffers, one is the mu4e-headers buffer and the other the mu4e-view buffer, then I reply to this email, I have now the mu4e-headers buffer and the mu4e-compose buffer. When I send my email I expect the mu4e-compose buffer to be killed once sent and my previous window configuration with mu4e-headers and mu4e-view buffers to be restored, instead I end up with 3 windows, the mu4e-headers on top, a previous buffer just under it (scratch here) and the mu4e-view at bottom. I could fix it temporarily with the patch below but it is really bad and may work only for my usage (windows) as it seems people are using many different window configs (frames, only one window etc...). Thus the function `mu4e--compose-setup` is overriding several message-*-actions hook which prevent users to fix these issues themselves without modifying source as I did. Also I had to delay with a timer the windows restoration. I send you here the patch which at least is good to localize the problem and fix it in a better way. Thanks. diff --git a/mu4e/mu4e-compose.el b/mu4e/mu4e-compose.el index 4a91a10d..1512c62d 100644 --- a/mu4e/mu4e-compose.el +++ b/mu4e/mu4e-compose.el @@ -748,20 +748,15 @@ Is this address yours?" (set-buffer-modified-p nil) (undo-boundary)) -(defun mu4e--maybe-delete-frame () - "Delete frame if there are multiple and current one has a single -window." - ;; XXX: this doesn't quite work; need a way to filter out - ;; the one _real_ frame I'm looking at; but I always get 3 or so. - ;; Only consider _real_ frames with some size - ;; (when (one-window-p) - ;; (let ((real-frames - ;; (seq-filter (lambda (frame);; only count live visible parent frames. - ;; (not (frame-parent frame))) - ;; (visible-frame-list)))) - ;; (when (> (length real-frames) 1) - ;; (delete-frame)))) - ) +(defun mu4e--reorder-windows-after-send () + (run-at-time + 0.1 nil + (lambda () + (walk-windows + (lambda (w) + (with-selected-window w + (unless (memq major-mode '(mu4e-headers-mode mu4e-compose-mode)) + (delete-window w)))))))) (defun mu4e--compose-setup (compose-type compose-func &optional switch) "Set up a new buffer for mu4e message composition. @@ -782,7 +777,7 @@ Optionally, SWITCH determines how to find a buffer for the message (mu4e-message-at-point))) (mu4e-compose-parent-message parent) (mu4e-compose-type compose-type) - (actions '(mu4e--maybe-delete-frame)) + (actions '(mu4e--reorder-windows-after-send)) (buf)) (advice-add 'message-is-yours-p :around #'mu4e--message-is-yours-p) (run-hooks 'mu4e-compose-pre-hook) ;; run the pre-hook. Still useful?

…

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.*Message ID: ***@***.***>

-- Thierry

[djcb]

I'll reopen this one...

[djcb]

FWIW, things work fine for me when sending / cancelling a message composition if I have just the headers / view message (ie., the canceled composition is replaced with the message-view I started with)

However, when there's another window, cancelling the composition strangely changes the window to the headers view (so I have two now). Have to find what causes the difference.

[thierryvolpiatto]

"Dirk-Jan C. Binnema" ***@***.***> writes:

1. ( ) text/plain (*) text/html FWIW, things work fine for me when sending / cancelling a message composition if I have just the headers / view message (ie., the canceled composition is replaced with the message-view I started with)

Same for me but in addition a third window is created between header and view windows. This window is displaying the last buffer used before staring Mu4e.

However, when there's another window, cancelling the composition strangely changes the window to the headers view (so I have two now). Have to find what causes the difference.

Here what I am using since 1 or 2 months now (but it is still not the right fix IMO even if working fine for me, see FIXME below): commit 7040590b2a8b62e3f509000c7c1ed201b3633483 Author: Thierry Volpiatto ***@***.***> Date: Sun Jan 7 14:57:54 2024 +0100 Restore frame config after sending email diff --git a/mu4e/mu4e-compose.el b/mu4e/mu4e-compose.el index 4a91a10d..6377f11c 100644 --- a/mu4e/mu4e-compose.el +++ b/mu4e/mu4e-compose.el @@ -748,21 +748,11 @@ Is this address yours?" (set-buffer-modified-p nil) (undo-boundary)) -(defun mu4e--maybe-delete-frame () - "Delete frame if there are multiple and current one has a single -window." - ;; XXX: this doesn't quite work; need a way to filter out - ;; the one _real_ frame I'm looking at; but I always get 3 or so. - ;; Only consider _real_ frames with some size - ;; (when (one-window-p) - ;; (let ((real-frames - ;; (seq-filter (lambda (frame);; only count live visible parent frames. - ;; (not (frame-parent frame))) - ;; (visible-frame-list)))) - ;; (when (> (length real-frames) 1) - ;; (delete-frame)))) - ) - +;; FIXME: Something is already trying to restore the underlaying windows/frames +;; that were here before starting composing Mail but fails to do so. As a +;; workaround we save the frame config before composing and then restore it +;; through `message-send-actions' but a better fix would be to fix previous +;; attempt to restore initial window/frame config. (defun mu4e--compose-setup (compose-type compose-func &optional switch) "Set up a new buffer for mu4e message composition. @@ -782,23 +772,21 @@ Optionally, SWITCH determines how to find a buffer for the message (mu4e-message-at-point))) (mu4e-compose-parent-message parent) (mu4e-compose-type compose-type) - (actions '(mu4e--maybe-delete-frame)) - (buf)) + (winconf (current-frame-configuration)) + (restore-winconf (lambda () (set-frame-configuration winconf))) + (actions (append (list restore-winconf) message-send-actions)) + (buf (mu4e--compose-setup-buffer compose-type compose-func parent))) (advice-add 'message-is-yours-p :around #'mu4e--message-is-yours-p) (run-hooks 'mu4e-compose-pre-hook) ;; run the pre-hook. Still useful? (mu4e--context-autoswitch parent mu4e-compose-context-policy) - (with-current-buffer - (mu4e--compose-setup-buffer compose-type compose-func parent) + (with-current-buffer buf (funcall (or switch (mu4e--compose-switch-function)) (current-buffer)) (unless (eq compose-type 'edit) (set-visited-file-name ;; make it a draft file (mu4e--draft-message-path (mu4e--message-basename) parent))) (mu4e--compose-setup-post compose-type parent) ;; handle closing of frames. - (setq-local ;;message-kill-actions actions - message-postpone-actions actions - message-send-actions actions) - (setq buf (current-buffer))) + (setq-local message-send-actions actions)) (switch-to-buffer buf)))

…

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.*Message ID: ***@***.***>

-- Thierry

[djcb]

Hmm, can't reproduce the erroneous behavior today. Perhaps something changed in emacs itself?

Ie., I have a headers-view window and an unrelated window. In the headers-view, I view, then reply to some message R; then close it C-c C-k. And we're back at the beginning. Not sure what changed.

There's still some issue with deleting frames though.

[djcb]

Following the steps:

How to Reproduce

    Open an Email from mu4e headers buffer.
    You have now two windows, the headers window and the view buffer where your mail is.
    Hit "W" or "R" to reply, you have now the headers window (same as before) and in place of the view buffer the composition buffer.
    Change your mind and hit C-c C-k.
    You have now only the headers window. This because mu4e-message-kill-buffer set the view buffer before killing the composition buffer and also it check for windows that are no more existing.

this works fine for me (with 1.12.4). For you too?

[thierryvolpiatto]

"Dirk-Jan C. Binnema" ***@***.***> writes:

Following the steps: How to Reproduce Open an Email from mu4e headers buffer. You have now two windows, the headers window and the view buffer where your mail is. Hit "W" or "R" to reply, you have now the headers window (same as before) and in place of the view buffer the composition buffer. Change your mind and hit C-c C-k. You have now only the headers window. This because mu4e-message-kill-buffer set the view buffer before killing the composition buffer and also it check for windows that are no more existing. this works fine for me (with 1.12.4). For you too?

No it doesn't, always the same behavior.

…

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

-- Thierry

[thierryvolpiatto]

Thierry Volpiatto ***@***.***> writes:

"Dirk-Jan C. Binnema" ***@***.***> writes: > Following the steps: > > How to Reproduce > > Open an Email from mu4e headers buffer. > You have now two windows, the headers window and the view buffer where your mail is. > Hit "W" or "R" to reply, you have now the headers window (same as before) and in place of the view buffer the composition buffer. > Change your mind and hit C-c C-k. > You have now only the headers window. This because > mu4e-message-kill-buffer set the view buffer before killing the > composition buffer and also it check for windows that are no more > existing. > > this works fine for me (with 1.12.4). For you too? No it doesn't, always the same behavior.

Further more, I now endup with three windows after replying, Mu4e is now only usable with patch thierryvolpiatto@94b7666 applied (probably this patch doesn't apply anymore on top of master without resolving conflict).

…

> — > Reply to this email directly, view it on GitHub, or unsubscribe. > You are receiving this because you authored the thread.

-- Thierry

[djcb]

Hmm, trying to repro the original "How to reproduce" scenario here.

Start emacs/mu4e (change paths as needed):

 emacs -Q --eval "(progn (add-to-list 'load-path \"~/Sources/mu/build/mu4e\") (setq mu4e-mu-binary \"~/Sources/mu/build/mu/mu\") (require 'mu4e) (mu4e))"

bu to go to some unread mails. Press R. Press C-c C-k works as expected.

I tried with emacs master and emacs 28.2; both work for me.

[thierryvolpiatto]

"Dirk-Jan C. Binnema" ***@***.***> writes:

1. ( ) text/plain (*) text/html Hmm, trying to repro the original "How to reproduce" scenario here. Start emacs/mu4e (change paths as needed): emacs -Q --eval "(progn (add-to-list 'load-path \"~/Sources/mu/build/mu4e\") (setq mu4e-mu-binary \"~/Sources/mu/build/mu/mu\") (require 'mu4e) (mu4e))" bu to go to some unread mails.

RET on a unreaded mail, now you have two windows, and only then press R or W.

…

Press R. Press C-c C-k works as expected. I tried with emacs master and emacs 28.2; both work for me. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

-- Thierry

[djcb]

Yeah, same in that case; two windows, I press R, the view window is replaced by the composition window. I press C-c C-k, and the composition goes away and we're back at the view window, just like before pressing R.

[thierryvolpiatto]

"Dirk-Jan C. Binnema" ***@***.***> writes:

1. ( ) text/plain (*) text/html Yeah, same in that case; two windows, I press R, the view window is replaced by the composition window. I press C-c C-k, and the composition goes away and we're back at the view window, just like before pressing R.

I tried your recipe and indeed C-c C-k is restoring the previous window conf, so I guess something in my setting is modifying this behavior, I checked and didn't find why, I have no code or variable directly modifying the window configuration behavior but because we are relaying on Gnus, I believe modifying either gnus-* or message-* or mm-* vars is modifying some piece of gnus code hence the behavior I have. This mean anyway that the current code is fragile and may fail in any other corner cases. I suggest we determine how you are currently fixing such issue in your code to be able to fix it, seems it is unclear currently. Also I opened a new issue (#2676) that concern not only window restoration after C-c C-k but also after sending, and with your recipe it is still not working, I endup with three windows after sending, so I guess you should either consider this in #2600 (seems you don't) or reopen #2676. The two issues are related, the good way to fix this IMO is to have common code that fix both issues. Hopefully, with last master I can advice mu4e--compose-setup without having to maintain a working branch. I still think saving and restoring frame/window conf is the right way to do. Without such advice mu4e is not usable for me actually (really annoying issue). (defun tv:advice-mu4e--compose-setup (compose-type compose-func &optional switch) (cl-assert (member compose-type '(reply forward edit new))) (unless (mu4e-running-p) (mu4e 'background)) ;; start if needed (let* ((parent (when (member compose-type '(reply forward edit)) (mu4e-message-at-point))) (mu4e-compose-parent-message parent) (mu4e-compose-type compose-type) (frameconf (current-frame-configuration))) (advice-add 'message-is-yours-p :around #'mu4e--message-is-yours-p) (run-hooks 'mu4e-compose-pre-hook) ;; run the pre-hook. Still useful? (mu4e--context-autoswitch parent mu4e-compose-context-policy) (with-current-buffer (mu4e--compose-setup-buffer compose-type compose-func parent) (unless (eq compose-type 'edit) (set-visited-file-name ;; make it a draft file (mu4e--draft-message-path (mu4e--message-basename) parent))) (mu4e--compose-setup-post compose-type parent) (funcall (or switch (mu4e--compose-switch-function)) (current-buffer)) (let ((actions (list (lambda () (set-frame-configuration frameconf))))) ;; handle closing of frames. (setq-local ;;message-kill-actions actions message-return-actions actions message-send-actions actions message-kill-actions actions)) (current-buffer))))

…

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

-- Thierry

[djcb]

Both sending and cancelling messages work fine for me, if it were "really annoying" surely I'd notice.

mu4e is at the mercy of what gnus gives up and the abstraction is not fully watertight, so perhaps you found something that influences things in a way that bubbles up.

I'm planning to rework the composition code a bit, so whatever your advising likely will go away. But hopefully it's not necessary anymore.

[thierryvolpiatto]

"Dirk-Jan C. Binnema" ***@***.***> writes:

1. ( ) text/plain (*) text/html Both sending and cancelling messages work fine for me, if it were "really annoying" surely I'd notice.

Not necessarily, we have all our own workflow and we sometimes miss obvious bugs or misfeatures.

mu4e is at the mercy of what gnus gives up and the abstraction is not fully watertight, so perhaps you found something that influences things in a way that bubbles up.

Exactly.

I'm planning to rework the composition code a bit, so whatever your advising likely will go away. But hopefully it's not necessary anymore.

Great, looking forward for this. Thanks.

…

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

-- Thierry

[theophilusx]

Another data point regarding new behaviour in latest version.

I'm often seeing a read/view buffer pop back up without being requested. I think I've narrowed down the exact steps required to reproduce.

1. Open my inbox (SPC j i in my case)
2. Move to some message in the header view
3. Hit on the message to read it
4. Hit 'q' whil in the message buffer to exist and return to the header view
5. Mark the message in some way i.e. 'd' to delete it
6. Hit 'x' to apply marks

at this point, the view buffer for the previously viewed message pops up again.

I then hit 'q' to close the view buffer and then hit 'q' again to exit the headers view and return to the main mu4e window.

I'm running Emacs 30.0.50 on Fedora 39 under either hyprland (Wayland) or i3 (X).
I use meow.el, so modal editing.
I have no frame or window settings in Emacs and don't use anything like winnum or any other window management packages. Essentially, I start emacs as a daemon and then use emacsclient full scren frame. I have a dedicatged virtual desktop for mu4e. When I want to do other stuff, I move desktop workspace (virtual desktop) and open a new frame.

The popping up of the view buffer after applying marks is a little irritating, but no big deal. It is behaviour I only noticed fairly recently. However, as I run both Emacs and mu4e from git HEAD, really don't know if this new behaviour is due to changes in Emacs or changes in mu4e.

Note that I see the same behaviour with Emacs build with standard GTK (for X) and with the pure or pgtk variant (for Wayland).

[thierryvolpiatto]

"Dirk-Jan C. Binnema" ***@***.***> writes:

1. ( ) text/plain (*) text/html Both sending and cancelling messages work fine for me, if it were "really annoying" surely I'd notice. mu4e is at the mercy of what gnus gives up and the abstraction is not fully watertight, so perhaps you found something that influences things in a way that bubbles up. I'm planning to rework the composition code a bit, so whatever your advising likely will go away. But hopefully it's not necessary anymore.

I think I found the cause of the bug; Basically, this is because at one point you are using switch-to-buffer to make a buffer current inside several with-current-buffer and even from a with-temp-buffer which return another buffer!!! (instead of the temp buffer!), this shuffle the window history and after each operation emacs lost what was the underlying buffer. Thus this make the code really hard to read, debug and follow, even more because message/gnus functions are involved from closures (and sometimes advised) and the reader has to follow this. Probably once this will be cleaned we will see clearly what's wrong.

…

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

-- Thierry

[djcb]

Yeah, I'm trying to make that convoluted path a bit less so; however, fundamentally mu4e handles buffer display, and it takes that task over from the Gnus code. That way, it can display buffers the way that mu4e does that, and support display-buffer etc.

Anyway, let's see how things work after changes.

[thierryvolpiatto]

"Dirk-Jan C. Binnema" ***@***.***> writes:

Yeah, I'm trying to make that convoluted path a bit less so; however, fundamentally mu4e handles buffer display, and it takes that task over from the Gnus code. That way, it can display buffers the way that mu4e does that, and support display-buffer etc. Anyway, let's see how things work after changes.

Seems better now with mu4e-compose-post-hook (4440594).

…

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

-- Thierry

[djcb]

Closing this ticket, I think it's about as good as it gets now :-) Thank all for the inputs.