GNOME Bugzilla – Bug 588077
entry completion should be triggered by idle callback
Last modified: 2018-04-15 00:01:15 UTC
Currently triggering happens by a 300ms timeout. This has some disadvantages: - I have to wait until I can interact with the completion. If I know how the completion is going to look, I want to use it without seeing it. Example: In the browser, I press these keys to open Slashdot: s l <down arrow> <return> This fails because I don't stop for 300ms between the l and the down arrow, so no completion has been popped up. The down arrow is ignored and the return key triggers a different action (Google search for "sl"). - The priority of triggering the completion got reduced. Now it's IDLE priority and not DEFAULT, which is better as popping up a completion is not more important than reacting to user input or redrawing.
Created attachment 138047 [details] [review] Bug 588077 – entry completion should be triggered by idle callback Previously, the callback was a timeout after 300ms. This has some disadvantages: - I have to wait until I can interact with the completion. If I know how the completion is going to look, I can use it without seeing it. Example: In the browser, I press these keys to open Slashdot: s l <down arrow> <return> This fails because I don't stop for 300ms between the l and the down arrow, so no completion has been popped up. The down arrow is ignored and a different action (Google search for "sl") is taken. - The priority of triggering the completion got reduced. Now it's IDLE priority and not DEFAULT, which is better as popping up a completion is not more important than reacting to user input or redrawing.
Created attachment 138048 [details] [review] Clean up code after using idle callback (Bug 588077) - Simplify the function that instantiates the callback, as we don't have to re-add it. - Rename variables from "completion_timeout" to "completion_idle"
I've tried the patch now, and it feels right to me. I honestly can't recall why we added the timeout initially. So, as far as I am concerned, this is ok to commit. Kris, do you recall why we have that timeout ?
(In reply to comment #3) > I've tried the patch now, and it feels right to me. > I honestly can't recall why we added the timeout initially. > So, as far as I am concerned, this is ok to commit. > > Kris, do you recall why we have that timeout ? Euuuh, good question. The only thing I can recall off head is that the completion should never get into the user's way. Either way, I've been digging in the history a bit as well and it seems this timeout originates from the original prototyping discussions back in 2000. Most people liked the Mozilla completion behavior back then (and this was still a Milestone build, boy I am getting old ...). What James Henstridge suggested back then: "" If you decide on the form of completion found in most web browsers these days, please do it similarly to mozilla, and put in a delay before performing the completion. It is really annoying to type in the exact string you want and press enter, only to find that the program has appended something extra onto the end. ""
As far as I understand James' remark, it only applies to inline completion, while my patch was strictly about popup completion. So I had a look at how inline completion works, and it starts an G_PRIORITY_HIGH idle handler in the entry's insert-text where it does the text inserting. So the code doesn't follow James' suggestion. That said, I don't even think he's right. You only want to use inline completion when you're picking from a set of valid values anyway, and completion only happens while you are having incomplete text entered, so immediate completion seems like the right option here.
Created attachment 138967 [details] [review] Optimization: Don't call gtk_entry_completion_complete() twice When inserting text, gtk_entry_completion_complete() was called twice, once in response to the "insert-text" signal, once for the "changed" signal. Remove one call. A side effect of this patch is a separation between the code paths that do inline completion and the ones doing popup completion.
(In reply to comment #5) > As far as I understand James' remark, it only applies to inline completion, > while my patch was strictly about popup completion. Yes. I think the original approach was to inline complete only when there was a single exact match, otherwise use the popup entry. > So I had a look at how inline completion works, and it starts an > G_PRIORITY_HIGH idle handler in the entry's insert-text where it does the text > inserting. So the code doesn't follow James' suggestion. It might have followed the suggestion originally -- a lot has happened between the year 2000 and now. > That said, I don't even think he's right. You only want to use inline > completion when you're picking from a set of valid values anyway, and > completion only happens while you are having incomplete text entered, so > immediate completion seems like the right option here. In case you are picking only from a set of valid values, I agree with you. But there are different possible scenarios. For a history-adding entry, you might want to accept another string that is not currently part of the completion set. In such a case, there should not be an immediate completion IMHO.
Hello gentlemen, any news of this bug ? Regards.
We're moving to gitlab! As part of this move, we are moving bugs to NEEDINFO if they haven't seen activity in more than a year. If this issue is still important to you and still relevant with GTK+ 3.22 or master, please reopen it and we will migrate it to gitlab.
As announced a while ago, we are migrating to gitlab, and bugs that haven't seen activity in the last year or so will be not be migrated, but closed out in bugzilla. If this bug is still relevant to you, you can open a new issue describing the symptoms and how to reproduce it with gtk 3.22.x or master in gitlab: https://gitlab.gnome.org/GNOME/gtk/issues/new