Differential D13517

Fix QuickOpen with Qt 5.11 (bug 395039)
ClosedPublic
Actions

Authored by brauch on Jun 13 2018, 2:33 PM.

Details

Reviewers
dhaumann
cullmann
mwolff
Commits
R40:291b68994d7c: Fix QuickOpen with Qt 5.11
Summary

Apparently the item model changes in Qt 5.11 cause our quickopen list to be empty.

I do not know why the old code does not work, to be honest, to me it seems okay. The only thing which is definitely wrong is the call to mapFromSource; the index passed to this belongs to the model which has since been replaced, and not to the actual model attached to the proxy model. That issue is unrelated though, commenting out that code (and thus removing the feature) doesn't fix it. With the suggested patch it's not an issue any more either because we don't change the model.

I like the suggested implementation better anyways, so I think we should change this in either case, but does anyone have an idea why the old code is broken? Is that maybe even a regression in Qt itself? @mwolff?

Diff Detail

Repository
R40 Kate
Lint
Lint Skipped
Unit
Unit Tests Skipped
brauch created this revision.Jun 13 2018, 2:33 PM
Restricted Application added a project: Kate. · View Herald TranscriptJun 13 2018, 2:33 PM
Restricted Application added a subscriber: kwrite-devel. · View Herald Transcript
brauch requested review of this revision.Jun 13 2018, 2:33 PM
brauch edited the summary of this revision. (Show Details)Jun 13 2018, 2:38 PM
mwolff accepted this revision.Jun 13 2018, 3:00 PM

no clue, I wouldn't be surprised by bugs/regressions in Qt - I've seen a lot of breakage in item/view code recently

This revision is now accepted and ready to land.Jun 13 2018, 3:00 PM
cullmann accepted this revision.Jun 13 2018, 3:04 PM

If that change works in old Qt and new, I am happy with it.

brauch added a comment.Jun 13 2018, 3:27 PM

I'll dig around a bit and see if I can find the change in Qt causing this. In the meantime, do you want this in applications/18.04 or master only?

brauch added a comment.Jun 13 2018, 3:41 PM

Looks like it was already fixed for Qt 5.11.1, e15fc26e9fdbff141890a3e2e8dc4ef935d022a0 sounds a lot like it:

commit e15fc26e9fdbff141890a3e2e8dc4ef935d022a0
Author: Christian Ehrlicher <ch.ehrlicher@gmx.de>
Date: Fri May 25 21:06:43 2018 +0200

QSortFilterProxyModel: don't assert when old model gets destroyed

When a new model was set with setSourceModel() and the mapping was
built up, the destruction of the old model caused a reset in the
QSortFilterProxyModel which lead to an empty view or an assertion.
Now we properly disconnect the old model again and also clean up the old
mapping/persistent indexes when a new source model is set.

Task-number: QTBUG-44962
Task-number: QTBUG-67948
Task-number: QTBUG-68427
Change-Id: I2e0612899c210bde3ac0cfa59aefd78269deee5b
Reviewed-by: David Faure <david.faure@kdab.com>

That also changes the code I think is relevant for this bug, i.e. QSortFilterProxyModel's setSourceModel() method.

Closed by commit R40:291b68994d7c: Fix QuickOpen with Qt 5.11 (authored by brauch). · Explain WhyJun 13 2018, 8:56 PM
This revision was automatically updated to reflect the committed changes.
cullmann added a comment.Jun 14 2018, 4:43 AM

Hmm, we want to have it in master, too, or?

brauch added a comment.Jun 14 2018, 6:50 AM

Yes sorry, I felt too tired yesterday to correctly resolve the version number merge conflict ;) Done now.

cullmann added a comment.Jun 14 2018, 6:54 AM

Nothing to be sorry about ;=)
Thanks for taking care!

Revision Contents
Changeset List

PathPackages
Mkate/katequickopen.cpp (21 lines)

Diff 36101

View Options

kate/katequickopen.cpp

Loading...