My favorites | Sign in
Project Home Downloads Wiki Issues Source
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 607: Autocomplete in reviewer field can miss quickly-typed chars
3 people starred this issue and may be notified of changes. Back to list
Status:  Released
Owner:  mf...@codeaurora.org
Closed:  Sep 2010


Sign in to add a comment
 
Reported by dsand...@google.com, Jun 23, 2010
Affected Version: 2.1.3

What steps will reproduce the problem?
1. Open a change.
2. Very rapidly type some part of the desired username.

What is the expected output? What do you see instead?

Sometimes, the last character doesn't get sent to the autocompletion, causing the drop-down to show inaccurate suggestions if there are other users in the system whose names stem from the first N-1 characters of what's been typed.

For example, if I rapidly type "jimmys" intending to get "Jimmy Stewart <jimmys@bedfordfalls.us>, the list of completion suggestions might display all the "jimmy" users that Gerrit knows about, as if I hadn't typed the final "s".

This can make it dangerous to quickly type a complete username (thinking it will autocomplete to exactly the person you want) and press tab and space; you'll add the wrong person to the review.

Please provide any additional information below.

Jun 24, 2010
#1 sop@google.com
(No comment was entered for this change.)
Status: Accepted
Labels: Milestone-2.1.4
Aug 4, 2010
#2 sop@google.com
(No comment was entered for this change.)
Labels: -Milestone-2.1.4
Sep 10, 2010
Project Member #3 mf...@codeaurora.org
This appears to be a non serialization issue in the way that SuggestBox is designed.  It dispatches every keystroke and forgets them.  The SuggestOracle then calls back the SuggestBox when it is ready with an answer.  But since the AccountSuggestOracle further dispatches the lookups via RPC, I believe that the RPC responses can return out of order (depends on the server response times).  If an earlier lookup returns after a later one (which is somewhat encouraged by the fact that earlier less constrained lookups likely have larger data sets and take longer to complete)
then, the earlier lookup results will replace the later results potentially displaying invalid results.  

It would be nice if SuggestBox kept track of requests and dropped any replies belonging to requests older than the last applied reply's request, but I am not sure if the GWT project would endorse such a "fix".
Sep 13, 2010
Project Member #4 mf...@codeaurora.org
Apparently it is a known issue.

https://code.google.com/p/google-web-toolkit/issues/detail?id=3341

My RPCSuggestOracle works by dropping old requests instead of limiting the amount of requests on the wire.  It seems that most browsers limit the wire requests to 2 anyway.  The problem with limiting that I didn't like is that it gives priority to the oldest request which is the one that is likely invalid now anyway since new characters have been typed.  Additionally, if typing from a blank state, each character is likely to limit the search criteria, the oldest response might simply take longer than (and even timeout) a simpler response created by a longer later query string.  Of course, ideally, older requests would be canceled upon receiving a newer response, but I cannot figure out how to do that with the current RPC mechanisms used by Gerrit.  The current GWT docs suggest that RPC calls can return a (http) Request object, this has a cancel method.  But Gerrit's mechanisms do not seem to support this. :(

Owner: mf...@codeaurora.org
Sep 15, 2010
#5 sop@google.com
 Issue 720  has been merged into this issue.
Sep 15, 2010
#6 sop@google.com
To support cancelling an active request, we'd need
to change gwtjsonrpc to permit the Request object
as a return value from an RPC method, rather than
requiring void.

That will look funny on the server side, because
we cannot create a GWT Request object there, but
the signature of the method still requires it as
the return value.  So we'd have to return null.

A short-term workaround might be to do what we do
in PatchScreen.  Have the Gerrit UI object keep a
counter, and don't call onSuggestionsReady if the
current counter doesn't match the current RPC.

See PatchScreen refresh() on line 362.
Sep 15, 2010
Project Member #7 mf...@codeaurora.org
Yeah, I was wondering if we might need to add the Request to the server side.  If so, I had the same idea, simply return null, not pretty, but it seems better than not canceling the unneeded request.

As for the short term workaround, it sounds like it will simply drop anything that is not the latest RPC.  That is what my proposed solution (not yet uploaded) does, but by simply comparing the request to the latest request (is there an advantage to keeping the count?).  So, if that is good solution, I will submit it.  If you eventually figure out how to allow us to cancel, I have a proposed update to my current solution that will enable that too.
Sep 15, 2010
#8 sop@google.com
Fixed by I33b2419cec5a8e9878c2eedb227b402bb9897455
Status: Submitted
Labels: FixedIn-2.1.6
Dec 15, 2010
#9 sop@google.com
(No comment was entered for this change.)
Status: Released
Sign in to add a comment

Powered by Google Project Hosting