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

Extended selection with Shift in QAbstractItemView fails if rows are added/removed/resorted

    XMLWordPrintable

Details

    • Bug
    • Resolution: Unresolved
    • P4: Low
    • None
    • 4.7.2, 5.4.0 Beta
    • Widgets: Itemviews
    • None
    • f1e90768095be37419ba4bf3c69ec5c860bdbcb6

    Description

      Consider the simple demo app which I've attached (test.cpp). If you click an item, then change the sort order, and then Shift-click another item, you would expect that all items between the two clicked items are selected. However, the starting point of the selection is not the item that has been clicked first, but the item that is now at the position of the first click. The same buggy behaviour can be reproduced without sort order changes if items are added or removed after the first click, such that the position of the first clicked item changes.

      This has been reported in Dolphin, and it may lead to data loss:
      https://bugs.kde.org/show_bug.cgi?id=262638

      The reason why it goes wrong is that QAbstractItemView keeps track of the starting point of the Shift-selection (also known as "current selection") in d->pressedPosition, a QPoint which is not updated when the position of the item changes. I propose to use a QPersistentModelIndex instead. I'm attaching a patch (including unit test) that fixes it for me.

      I haven't filed a merge request because my patch breaks an existing unit test (tst_QTableView::taskQTBUG_4516_clickOnRichTextLabel). In my opinion, this failure is not my fault, but it is due to another bug. It can be worked around by either

      a) showing the QTableView in the unit test,
      b) calling the table view's visualRect() member (for ANY index), or
      c) calling the table view's indexAt() member (for ANY position)

      before the mouse click is made in the test.

      Further investigations have shown that the position of the mouse press event that the item view gets is wrong - calling setIndexWidget() is apparently not enough to make the label fully aware of its new position inside the item view. The reason why it worked before my patch is that the unit test calls the item view's setCurrentIndex() member, which calls visualRect() without my patch, triggering workaround b), see above. You can see that by removing that call (and the following QCOMPARE) from the unit test -> it fails, just like it fails if my patch is applied.

      I wasn't sure what the best way to resolve this issue in the unit test is (either work around it, as I've described above, or make sure that the widget knows its correct position after setIndexWidget() is called), so I thought I'd better let you know what I found out so far.

      Reopened on 2016-06-09 because of the issue that was mentioned in the most recent comments.

      Attachments

        1. test.cpp
          4 kB
          Frank Reininghaus

        Issue Links

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

          Activity

            People

              freininghaus Frank Reininghaus
              freininghaus Frank Reininghaus
              Votes:
              2 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated:

                Gerrit Reviews

                  There are no open Gerrit changes