#1575 Mouse double click selects full line

[mickey]

I have a small Qt 5.2 project using the provided Qt bindings of Scintilla 3.3.9. When I double click in the component the full line gets selected.

Connecting to the doubleClick signal, doesn't help either, the selection happens after the event.

[Neil Hodgson]

This appears to be a difference in mouse event handling from 4.x since the current code handles double click OK with 4.7 and 4.8 but with 5.x treats it as a triple click.

On Qt 4.x a double click is seen as two events: a mousePressEvent and then a mouseDoubleClickEvent. On 5.x there are three events: two occurrences of mousePressEvent then a mouseDoubleClickEvent.

The code could be modified with a Qt version number #if. There may also some other way of interpreting the events.

I'll accept a well-written patch that works on both 4.x and 5.x. If no one else gets to it, I may work on this but not for a while.

[mickey]

What about:

diff -r 4ad65fd34b83 qt/ScintillaEditBase/ScintillaEditBase.cpp
--- a/qt/ScintillaEditBase/ScintillaEditBase.cpp    Thu Feb 06 09:02:21 2014 +1100
+++ b/qt/ScintillaEditBase/ScintillaEditBase.cpp    Fri Feb 07 12:52:02 2014 +0100
@@ -310,11 +310,15 @@
    emit buttonReleased(event);
 }

+#if QT_VERSION < QT_VERSION_CHECK(5, 0, 0)
 void ScintillaEditBase::mouseDoubleClickEvent(QMouseEvent *event)
 {
    // Scintilla does its own double-click detection.
    mousePressEvent(event);
 }
+#else
+void ScintillaEditBase::mouseDoubleClickEvent(QMouseEvent *) {}
+#endif

 void ScintillaEditBase::mouseMoveEvent(QMouseEvent *event)
 {

I put the whole method in an #ifdef to get rid of the unused warning of the "event". Works well with my setup. Thanks for pointing me to the problem.

[Neil Hodgson]

Committed with reformat as [6b9cc8].

[mickey]

Well I thought about it. To me it feels the current behaviour is not very Qt-like.

The events are passed to the user e.g. by

void ScintillaEditBase::mousePressEvent(QMouseEvent *event)
{
     ...
     emit buttonPressed(event);  
     ...
}

but usually you communicate via the accepted flag of a event, if it was handled. One could just add a early return here, if the event was accepted, but then in the mouseReleaseEvent the emit comes last (for symmetry reasons I suppose), so the user cannot intercept the release handling of Scintilla.

One could have two signals (beginning and end) in each mouse event and return early, if the beginning one is accepted. Maybe even more Qt-like, one could leave these signals out entirely and rely on the event filter mechanism of Qt, which allows the user do decide which events are passed to Scintilla.

At lease I would start accepting handled events like this (incorporates my former patch):

diff -r 4ad65fd34b83 qt/ScintillaEditBase/ScintillaEditBase.cpp
--- a/qt/ScintillaEditBase/ScintillaEditBase.cpp    Thu Feb 06 09:02:21 2014 +1100
+++ b/qt/ScintillaEditBase/ScintillaEditBase.cpp    Fri Feb 07 14:49:53 2014 +0100
@@ -274,7 +274,8 @@
                    pos, false, false, sqt->UserVirtualSpace());
        sqt->sel.Clear();
        sqt->SetSelection(selPos, selPos);
-       sqt->PasteFromMode(QClipboard::Selection);
+       sqt->PasteFromMode(QClipboard::Selection);            
+       event->accept();
        return;
    }

@@ -292,6 +293,7 @@
 #endif

        sqt->ButtonDown(pos, time.elapsed(), shift, ctrl, alt);
+       event->accept();
    }
 }

@@ -313,7 +315,10 @@
 void ScintillaEditBase::mouseDoubleClickEvent(QMouseEvent *event)
 {
    // Scintilla does its own double-click detection.
+#if QT_VERSION < QT_VERSION_CHECK(5, 0, 0)
    mousePressEvent(event);
+#endif
+   event->accept();
 }

 void ScintillaEditBase::mouseMoveEvent(QMouseEvent *event)

[Jason Haslam]

So this ticket is starting to conflate different issues. I agree that defining away the mouseDoubleClickEvent on Qt 5 is probably the right fix.

I can't comment on the signals that take event parameters. Those don't look familiar to me. The signals were mostly meant to implement Scintilla notifications.

What's the purpose of adding the accept calls? What problem does it solve?

[mickey]

Well, it depends what the Qt integration should be. If there is the vision of a Scintilla Qt component, which fits seamless in the Qt eco system, then using their event style is very important. So, adding the accept calls was a first very small step. For just resolving the bug, the first patch could do it. (I don't know the full Scintilla side of course, and just a bit of the Qt side...)

[Jason Haslam]

I try to avoid calling accept or ignore unless there's some good reason for it. I believe that events are accepted by default so calling accept explicitly is usually unnecessary.

[Neil Hodgson]

Qt 5.3 reverts to 4.x behaviour so this patch was reverted with [56de49].