msgstore: restore the directoryChange bool to fix new message bell

In 24eab0f5ef63 the condition under which the triggerDirectoryChange
callback is executed was changed so that it would now only run if there
were new messages in the currently scrolled into view part of the
message list whereas previously it ran for new messages anywhere in the
list.

The motivation for this was to make the mail-received hook work even
when the tab of the receiving account is not currently focused by
fetching the new message's/s' headers needed for the hook to execute
based on what changed in the scrolled into part of the list. This
limitation is needed because apparently we don't currently have a better
way to recognize new messages other than comparing the list of UIDs
provided by the worker and that kept by the store.

My current understanding is that we cannot remove the scroll view limit
because then we would be fetching headers for all messages in the
directory which can be a somewhat intensive operation. However we can
disregard the view limit for the bell as that does not require the
headers: it only cares about that something has changed.

It is noteworthy that the bell is rung on aerc startup: I assume that
this occurs with the initial load of the message store for every account
when the else branch where directoryChange is set to true inevitably
executes.

Overall, this patch is more of a workaround than a proper fix: the ideal
situation would be if we were able to fetch headers for new messages
independently of the scroll status. However as this is how this was
before, it should be suitable as a temporary solution.

There are also further problems here: currently we have triggerNewEmail
and triggerDirectoryChange callbacks which are both supposed to run when
new mail is received, the latter only implicitly. And yet they both use
a different mechanism and thus execute under different circumstances. It
would be ideal to move the bell into the new mail trigger and get rid of
the directory change one as it is otherwise unused. However this is not
done here because for some setups the new mail trigger does not run
until the tab is focused which is what the aforementioned commit was
meant to fix but apparently succeeded in doing so only partially. The
directoryChange trigger does not have this drawback and thus should be
kept until the issue is resolved for all setups.

Also note that for instance for O365 mail the bell still doesn't work
properly as there new mail only appears in the store after the given
folder is reopened (:prev-folder :next-folder) and the bell is thus rung
only then.

Fixes: 24eab0f5ef63 ("msglist: fetch headers even when not focused")
Fixes: https://todo.sr.ht/~rjarry/aerc/249
Changelog-fixed: Restore previous behaviour of the new message bell
 which was broken in the last two releases for at least some setups.
Signed-off-by: Karel Balej <balejk@matfyz.cz>
Acked-by: Robin Jarry <robin@jarry.cc>

1 files changed, 7 insertions, 0 deletions

diff --git a/lib/msgstore.go b/lib/msgstore.go
index f1399e4e..8f626eb8 100644
--- a/lib/msgstore.go
+++ b/lib/msgstore.go
@@ -249,6 +249,7 @@ func (store *MessageStore) Update(msg types.WorkerMessage) {
 	var newUids []uint32
 	update := false
 	updateThreads := false
+	directoryChange := false
 	start := store.scrollOffset
 	end := store.scrollOffset + store.scrollLen
 
@@ -263,6 +264,7 @@ func (store *MessageStore) Update(msg types.WorkerMessage) {
 				newMap[uid] = msg
 			} else {
 				newMap[uid] = nil
+				directoryChange = true
 				if i >= start && i < end {
 					newUids = append(newUids, uid)
 				}
@@ -287,6 +289,7 @@ func (store *MessageStore) Update(msg types.WorkerMessage) {
 				newMap[uid] = msg
 			} else {
 				newMap[uid] = nil
+				directoryChange = true
 				if i >= start && i < end {
 					newUids = append(newUids, uid)
 				}
@@ -381,6 +384,10 @@ func (store *MessageStore) Update(msg types.WorkerMessage) {
 		store.update(updateThreads)
 	}
 
+	if directoryChange && store.triggerDirectoryChange != nil {
+		store.triggerDirectoryChange()
+	}
+
 	if len(newUids) > 0 {
 		store.FetchHeaders(newUids, nil)
 		if store.triggerDirectoryChange != nil {