Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-52448

QQuickAbstractButton: clicked() called before nextCheckState()

    XMLWordPrintable

Details

    • Bug
    • Resolution: Done
    • P3: Somewhat important
    • 5.7.0 Beta
    • 5.6.0
    • Quick: Controls 2
    • None
    • Ubuntu 14.04.4LTS
    • 571f4be253386cb30a55060bed7d81ef356124eb

    Description

      With the stable, non-labs version of qabstractbutton.cpp, clicking a button results into its state being updated with nextCheckState(), then the released() and clicked() signals emitted:

      void QAbstractButtonPrivate::click()
      {
          // ...
          QPointer<QAbstractButton> guard(q);
          if (changeState) {
              q->nextCheckState();
              if (!guard)
                  return;
          }
          // ...
          if (guard)
              emitReleased();
          if (guard)
              emitClicked();
      }
      

      Good. Now, with the new, labs version of qquickabstractbutton.cpp, the opposite is done: first the released() and clicked() signals are emitted, then the button state is updated with nextCheckState():

      void QQuickAbstractButton::mouseReleaseEvent(QMouseEvent *event)
      {
          // ...
          if (wasPressed) {
              emit released();
              emit clicked();
          } else {
              emit canceled();
          }
      
          if (contains(event->pos()))
              nextCheckState();
          // ...
      }
      

      This means getting the state in an onClicked handler will present me the old, unchanged state. To me this looks like a serious regression:

      1. This is a breaking change from QtQuick controls 1.
      2. It seems to me that client code relying on onClicked likely wants to work with the new state resulting from the click, not the old one. Wrong?
        • Not an argument per se but incidentally, emitting the event at the very end and exposing the new state is the model used by JS events on the web, to which many developers are used to.
      3. Thinking it was maybe intentional, I tried to use the new behavior. In my case I was working on moving checkboxes (which inherit from Q(Quick)AbstractButton) from qtquick1 to qtquick2. So I moved my handlers from onClick to onCheckedChanged.
        1. But then, that means I can no longer set checked directly or I have a binding loop.
        2. Alright, let's rather Component.onCompleted: {checked = "...";}
        3. But then at startup, checked is changed, calling checkChanged, and negating the initial state of my checkboxes :-/

      → Am I missing something? Is this change intentional? If yes, what would be the proper way to do initialization and event handling for my checkboxes?

      Attachments

        Issue Links

          No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

              Unassigned Unassigned
              ronjouch Ronan Jouchet
              Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  There are no open Gerrit changes