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

[REG 5.13.0->5.13.1] QWidget::setFocusProxy steals (and breaks) focus from widgets at the end of the proxy chain

    XMLWordPrintable

Details

    • 23b998fa454ca021aa595f66d2e1964da4a119a4 (qt/qtbase/dev) 6ba52532810e928dd844c7e62d2ff72ffdfd2cd2 (qt/qtbase/5.15) 79008da82d9b5bcb99df88ff8b9dc5e130675e53 (qt/qtbase/5.15)
    • Bug Fixing Week Q2/2020, Bug Fixing Candidates

    Description

      Bug is reproduced with the attached example code:

      1. Compile and run the attached code.
      2. Wait a second for the second line edit to be created. Note the visual states of the two line edits.
      3. Type something. Note where the text goes.
      4. Click the window in between the line edits (so it would get click focus). Note the visual state.
      5. Type something again. Note where the text goes.

      Expected: The visual state remains consistent. The first line edit begins with focus and typing goes there. Clicking the window (step 4) moves focus to the second line edit and typing now goes there.

      Actual: The visual state remains very confusing. Both line edits have blinking cursors, the second line edit is highlighted for focus, but typing goes to the first line edit. Clicking the window (step 4) doesn't change anything; visuals are still wrong and the typing still goes to the first line edit.

      In short: If a widget A has a focus proxy B and B has the focus, then calling A->setFocusProxy(C) will cause the focus to be (partially) transferred to C. The "partial" part is a bug in itself, but is already reported in QTBUG-79707.

      Changing the focus proxy for a widget should not steal the focus from widgets further down the proxy chain if they have the focus.

      This is a regression introduced in 5.13.1 and is still present in 5.14.2.
      Changing focus proxies works in 5.13.0. The above steps yield the expected behaviour.

      The regression is almost certainly introduced by commit 3e7463411e549100eee7abe2a8fae16fd965f8f6, but I do not have a Qt build set up to verify that. The follow-up commit 947883141d9d8b3079a8a21981ad8a5ce3c4798e does not fix the issue; it just tries to steal the focus more properly (and is not quite there yet, as per QTBUG-79707).

      I have two suggestions:

      1. I believe commit 3e74634 tries to maintain the invariant that QApplicationPrivate::focus_widget always points to the last widget in a focus proxy chain, so setFocus(), hasFocus() and clearFocus() work correctly. To do this it would be sufficient to transfer the focus down from a widget during setFocusProxy if it currently is the focus_widget. Focus should not change when the focus_widget is already further down the proxy chain from where we're changing the chain. That would fix this bug.
      2. In the one case where we should transfer focus down, it has to account for all the relevant focus state. Currently QApplicationPrivate::setFocusWidget() is used directly, but this omits updating other parts of the internal focus state as QWidget::setFocus() would have done. In addition, the reason provided as argument is Qt::NoFocusReason, which (contrary to the 9478831 commit message) doesn't send FocusEvent's to the involved widgets and doesn't emit the QApplication::focusChanged() signal.

      It would also be good to update the QWidget::setFocusProxy() documentation to inform that focus will immediately change under certain conditions.

      Attachments

        1. CMakeLists.txt
          0.4 kB
        2. focus_proxy_usecase.tar.gz
          5 kB
        3. main.cc
          0.8 kB
        4. main.cc
          2 kB

        Issue Links

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

          Activity

            People

              vhilshei Volker Hilsheimer
              mkolsen Mathias Kaas-Olsen
              Votes:
              4 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews