#1575 Mouse double click selects full line

Bug
closed-fixed
Neil Hodgson
5
2014-03-21
2014-02-07
mickey
No

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.

Discussion

  • Neil Hodgson
    Neil Hodgson
    2014-02-07

    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.

     
  • Neil Hodgson
    Neil Hodgson
    2014-02-07

    • labels: --> scintilla, qt
    • status: open --> open-accepted
    • assigned_to: Neil Hodgson
     
  • mickey
    mickey
    2014-02-07

    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.

     
    Last edit: mickey 2014-02-07
    • Neil Hodgson
      Neil Hodgson
      2014-02-10

      Committed with reformat as [6b9cc8].

       

      Related

      Commit: [6b9cc8]

  • mickey
    mickey
    2014-02-07

    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
    Jason Haslam
    2014-02-07

    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
    mickey
    2014-02-08

    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
    Jason Haslam
    2014-02-08

    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
    Neil Hodgson
    2014-02-10

    • status: open-accepted --> open-fixed
     
  • Neil Hodgson
    Neil Hodgson
    2014-03-21

    • status: open-fixed --> closed-fixed