https://bugs.kde.org/show_bug.cgi?id=427672

--- Comment #17 from Nate Graham <n...@kde.org> ---
Git commit a458e4c7a7b02bf533f463d1be19138e195d8d22 by Nate Graham, on behalf
of Eduardo Cruz.
Committed on 08/02/2022 at 16:58.
Pushed by ngraham into branch 'Plasma/5.24'.

Avoid sorting old results based on new query input string

`SortProxyModel` reorders the results provided by krunner. It uses the query
input string to judge the associated results and sort them. It works great,
however we have a little bug that causes a nonsensical list to be shown for the
a few milliseconds right after we input a new query string.

At the time `setQueryString()` is called, we still haven't received any matches
result list from krunner for this new query. Milou still has the previous
query's results, which are completely dissociated from the new query input.

So at this point it was  doing its logic using the old results list from the
previous input string, but evaluating them against the newly provided input
string, which resulted in bogus behavior.

This example might make it more clear:

1 - We type string "fire" and wait for the query to finish and the final
results are displayed. We get this result set:

![Screenshot_20211227_055336](/uploads/3a2711a4f873dd619ab7f0d34bc93135/Screenshot_20211227_055336.png)

2- We type one more letter, "f", so now we have "firef" as input.
`SortProxyModel::setQueryString()` would set "firef" as its new `m_words` and
it would call its own `invalidate()`, however at this moment the results list
for "firef" haven't been provided by krunner yet: Milou still has the results
list for "fire", our previous query.

So `SortProxyModel` is processing the results from "fire" using the string
"firef" as criteria to judge its sorting, and this results in a quick
exhibition of this bogus list to the user. 

This is only shown for 250ms. It took some tries to take a screenshot of it,
but here it is:

![Screenshot_20211227_055504](/uploads/de49b9e0ea584f9f90867cff9b1ca704/Screenshot_20211227_055504.png)

As we can see, we have some bogus results like "Command line: run fire" and
"System Settings: Firewall" that have no business being there for the input
string "firef". They are there because they are still the results from the old
"fire" query. Also the ordering is different from the first list since the
proxy judged the relevancy in a different manner because it's evaluating that
list against the string "firef" and not "fire".

3 - 250ms later, krunner will have provided its results for "firef", and
`SortProxyModel` gets in action again, now sanely sorting the "firef" result
against its associated "firef" input string, and we get this final correct
list:

![Screenshot_20211227_055527](/uploads/af0dce0e73aa62225e2bdc89eabb56ed/Screenshot_20211227_055527.png)

So I fixed it by not calling `SortProxyModel::setQueryString()` anymore when we
have a query string change. Now we call it whenever we have a matches result
set change. So the `SortProxyModel` will maintain the old query string until we
receive the first result set for the new query string. That method's inner
logic will detect whenever a true query string change happened and will
properly call `invalidate()` when necessary.

With this patch applied, the bogus list in step 2 above will never be generated
and it won't be shown to the user.


(cherry picked from commit 58728acc19933a3d2a93d8cecfc77606dbf8dda9)

M  +7    -1    lib/resultsmodel.cpp
M  +2    -0    lib/runnerresultsmodel.cpp
M  +2    -0    lib/runnerresultsmodel.h

https://invent.kde.org/plasma/milou/commit/a458e4c7a7b02bf533f463d1be19138e195d8d22

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to