1720968 - Unfinished drag-and-drop after appearing of MessagePane in Wide View

Status: NEW

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Dmitry Butskoy (:buc)]

In short:

On the Wide View, if MessagePane was not shown before, it can cause to shrink folderPane and then occupies the bottom of previous folderPane's area. Thus, starting to leftclick on the folderPane, the releasing can occur already on the messagePane, which on practice leads to hidden unfinished drag-and-drop.

Comment 1

[Dmitry Butskoy (:buc)]

Steps to reproduce:

Choose View-->Layout-->Wide View .

Make sure the content of the folder pane spans enough lines to cause scrollbar to appear (ie. unexpand something if needed).

Choose some "server" line in the folder pane, fe. "Local Folders".
When you choose a "server", there are only two panes with a vertical separator -- folder pane and the account info.

Click on the lowest folder.
The folder will be selected, the thread pane and message pane appear.
The folder pane is shrinked (because of 'Wide View"), scrolled for selected folder to be shown, scrollbar appears etc.
The space previously occupied by the bottom of the folder pane is now occupied by the (left part of) message pane.

Note, that when you start to click the mouse, it was over the folder pane, but now, despite was not moved at all, it is over the message pane.

Now start to move the mouse, without any clicking, towards the folder pane, then reach it.
When you will be over the folder pane (somethimes even near it), you will mention that drag-and-drop process is active.
Click somewhere in the folder pane to stop this dragging.

Comment 2

[Dmitry Butskoy (:buc)]

The key place is here. The problem gone if change "onselect" to "onclick", but then the thread pane expanding is not performed after the start (and after the key movement).

It seems that there is a mess in the order of "onselect", mouse and dragging events. "Onselect" seems happen there even before the "mousedown", then the message pane appears somethimes between "mousedown" and "mouseup".

Comment 3

[Dmitry Butskoy (:buc)]

An easy and reliable solution seems to be just to deny dragging in this particular case.

It is unlikely and unconvenient to perform dragging in the small enough left-top pane, instead of, say, full height pane (after message pane is collapsed by F8). And it is a rare enough operation to dnd the whole folders on practice.

Such a denying does not affect a possibility to drop something into folder pane (aka moving messages between folders).

So the changes could be:

--- comm/suite/mailnews/content/folderPane.js.orig      2021-07-17 03:43:38.700088370 +0300
+++ comm/suite/mailnews/content/folderPane.js   2021-07-17 03:43:50.292016027 +0300
@@ -659,6 +659,15 @@ let gFolderTreeView = {
   },

   _onDragStart: function ftv_dragStart(aEvent) {
+    // Deny drag-and-drop on Wide View with message pane uncollapsed.
+    // If MessagePane was not shown before, it can cause to shrink folderPane
+    // and then occupies the bottom of previous folderPane's area.
+    // Thus, starting to leftclick on the folderPane, the releasing can occur already
+    // on the messagePane, which  on practice leads to hidden unfinished drag-and-drop.
+    if (Services.prefs.getIntPref("mail.pane_config.dynamic") == kWideMailLayout &&
+        !IsMessagePaneCollapsed())
+      return;
+
     // Ugh, this is ugly but necessary.
     let view = gFolderTreeView;

Comment 4

[Dmitry Butskoy (:buc)]

Run with MOZ_LOG=nsDragService:5 is useful here (but probably for gtk3 only).

Comment 5

[Nuno Silva [:njsg]]

I can reproduce this with a SeaMonkey 9b1pre build, as well as with Thunderbird 78.11.0 packaged by Debian, both amd64 linux.

Comment 6

[WaltS48 [:walts48]]

I can reproduce this using the Fedora build of Thunderbird 78.11.0, and using the Thunderbird build of 78.12.0 on Fedora 34 Workstation, Gnome 40 desktop.

Could not reproduce using TB 78.12.0 on Windows 10.

Comment 7

[Dmitry Butskoy (:buc)]

For gtk, a special mHiddenWidget is created, and probably all the drag operations are performed over it. If yes, it could explain why no one notices the change of the (normal, unhidden) widget under the mouse.

It is likely a rare combination -- first, "mouse is released already being over another widget, but without any move", second "the source selection for dragging (a selected row) is moved (scrolled) because the source widget's height is decreased".

Usually such a rare things receive "rewrite your application" verdict from an upstream. Esp. considering a comment in some related upstream gtk bug 728183.

So it seems the workaround should go further.

Comment 8

[Dmitry Butskoy (:buc)]

Similar symptome a decade ago: bug #736811 comment #34

Comment 9

[Dmitry Butskoy (:buc)]

Browser bug #784785 (with the same symptome) reproduced with 2.53.8 (CentOS-7 Linux, gtk3).

It could be fine to know whether some other similar cases exist, and how often they can appear on practice. Maybe some "modern" site too (ie. not chrome only).

Comment 10

[Dmitry Butskoy (:buc)]

Well,
mozilla/dom/events/EventStateManager.cpp is responsible for drag starting.

On the mouse press, it calls method BeginTrackingDragGesture(). Then, while the mouse is moved (if any), it calls method GenerateDragGesture(), which checks whether the mouse is moved "enough", and if so, starts the dragging. On the mouse release (which it receives when no drag started), it calls method StopTrackingDragGesture().

So, if you press the mouse and move it a bit, the drag is started. If you haven't moved the mouse (or you haven't moved it enough) and release it, the drag is not started.

It is obvious that on the mouse release any tracking drag gesture must stop. (If the drag is already started, the mouse release event seems to be catched by an underlying level (gtk, x11 etc.) and never reaches the EventStateManager code).

The problem is that for some (rare) cases, the mouse release event is received by ANOTHER EventStateManager instance. (It is the case for the MessagePane/WideView issue and for bug #784785 as well).

Fortunately, this situation can be detemined at eMouseUp time, by cheking whether the current ESM (ie., "this") is the same as (previously saved at eMouseDown time) "Active ESM".

So this change fixes both the issues:

--- mozilla/dom/events/EventStateManager.cpp.orig       2021-08-17 20:56:14.484483225 +0300
+++ mozilla/dom/events/EventStateManager.cpp    2021-08-17 20:56:23.604425650 +0300
@@ -657,16 +657,19 @@ EventStateManager::PreHandleEvent(nsPres
   }
   case eMouseUp: {
     switch (mouseEvent->button) {
       case WidgetMouseEvent::eLeftButton:
         if (Prefs::ClickHoldContextMenu()) {
           KillClickHoldTimer();
         }
         StopTrackingDragGesture();
+        if (sActiveESM && sActiveESM != this) {
+          sActiveESM->StopTrackingDragGesture();
+        }
         sNormalLMouseEventInProcess = false;
         // then fall through...
         MOZ_FALLTHROUGH;
       case WidgetMouseEvent::eRightButton:
       case WidgetMouseEvent::eMiddleButton:
         SetClickCount(mouseEvent, aStatus);
         break;
     }

Considering StopTrackingDragGesture() itself (just two pointers are zeroed), it spoils nothing. And it looks anyway that the mouse release should stop tracking drag gestures wherever it is.

Comment 11

[Dmitry Butskoy (:buc)]

There is an interesting fragment in mozilla/layout/base/PresShell.cpp, which sometimes changes things to follow "active" ESM. If you change the current logic if (check (A,B)) to if (check (A,B) || check (B,A)), this fixes the issue too (but then SM crashes when you try to login to bugzilla :) ).

This code block initially appeared by bug #603550 comment #7 in the form of "check(A,B) || check(B,A)", but then in bug #603550 comment #8 they have found that something goes wrong, and finally in bug #603550 comment #17 the block became the same as nowadays, with a comment added.

After the past 11 years, such a "|| check(B,A)" is probably not easy to add, but then perhaps something like && aEvent->mMessage == eMouseUp could help etc.

Just thoughts how the issue could be fixed in a more general way...

Comment 12

[Dmitry Butskoy (:buc)]

Attachment: ESM-logs.patch

A way to temporary add logs stuff into EventStateManager.cpp.

Run with MOZ_LOG="EventStateManager:5" environment set.

In the current form tracks all the events (including mouse moving) and the corresponf ESM.

For mouse DOWN/UP, prints the current and the active (if any) ESM instance.

Comment 13

[Dmitry Butskoy (:buc)]

It is unclear why this problem does not appear under Windows. This can be due to either some other order of events, or some low-level side effects from the implementation on gtk.

It would be nice to figure it out, to make sure there are no any hidden underlying bugs in the gtk system.

WaltS48,

Could you please discover a bit, whether there are some noticeable differences in behavior between Linux and Windows here, with different variations of the initial test?

Whether message pane appears right after the mouse press as well (or maybe after release)?

What happen if you do not release the mouse at all, but start to move it?

The message pane might be empty (yet nothing chosen in thread pane after the program start), or has a message (then the mouse UP event can be either over headers or over body). This could potentially change behavior.

And so on...

Comment 14

[WaltS48 [:walts48]]

No noticeable differences that I can determine.