548476 - Autocomplete window magically appears after autocompletion

Status: RESOLVED FIXED

(Camino Graveyard :: Location Bar & Autocomplete, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

On 1.9.2, bug 505263 doesn't seem to be fixed.

When pasting a URL (or editing an existing URL) and hitting return, Camino appears to do one of the following things:

1) load the last-autocompleted URL instead
2) something else (I think load the first URL that starts to match your paste; fuzzy on this manifestation at the moment)
3) load the desired URL and show the autocomplete window after loading

It's possible there are two or more bugs here, but I kind of think all three behaviors are related.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I think we've been speculating that the history queries behind autocomplete are slower on 1.9.2, so I'm wondering if the timing is off due to the slowness; that is, the return is getting sent to something in the autocomplete results that are still being generated at the time return is pressed, rather than the autocomplete results that are visible?

Comment 2

[Dan Weber]

That sounds likely. However, I'm almost positive that when bug 506345 finally gets fixed, this will disappear, so I'm focusing the (little) time I have available on that bug right now.

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I'm not sure what this is related to (I didn't notice it until four hours after it had appeared); perhaps this bug, perhaps not.

Camino[14412] Warning *** NSRunLoop ignoring exception 'Can't do regex matching, reason: Can't open pattern U_MALFORMED_SET (string http://tinderbox.mozilla.org/showbuilds.cgi?tree=Camino, pattern .*://(www\.)?forums/[.*, case 1, canon 2)' that raised during posting of delayed perform with target 0x154736a0 and selector 'processNextSearchChunk'

Comment 4

[philippe (part-time)]

(In reply to comment #3)
> I'm not sure what this is related to (I didn't notice it until four hours after
> it had appeared); perhaps this bug, perhaps not.
> 
> Camino[14412] Warning *** NSRunLoop ignoring exception 'Can't do regex
> matching, reason: Can't open pattern U_MALFORMED_SET (string
> http://tinderbox.mozilla.org/showbuilds.cgi?tree=Camino, pattern
> .*://(www\.)?forums/[.*, case 1, canon 2)' that raised during posting of
> delayed perform with target 0x154736a0 and selector 'processNextSearchChunk'

You probably accidentally (?) typed a '['. I see similar logging quite regularly when I hit the '[' instead of the return key. Here is one (and, no, I wasn't looking for tinderbox.m.o, I had typed 'tool'):
3/5/10 5:23:28 PM	Camino[76749]	Can't do regex matching, reason: Can't open pattern U_REGEX_MISSING_CLOSE_BRACKET (string http://tinderbox.mozilla.org/Camino/, pattern .*://(www\.)?tool[.*, case 1, canon 2)

Comment 5

[Christopher Henderson]

Attachment: Fix v1.0

Explicitly cancel the search when the URL is opened from the AutoCompleteTextField.

It fixes the problems that I've been seeing, but I'm sending it to Smokey first to confirm. If anyone else finds regressions, I'd be interested too.

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

This doesn't fix the paste-return-loads-previous-page-instead case (which is either point 1 or point 2, depending on what the previous was), but so far I haven't seen a single case of the stray autocomplete window appearing after an action, so it's definitely an improvement.

I haven't done much/any URL editing yet, so I'll try to remember to do some of that today.

(And, frankly, not seeing the @#$@#$@# window is so much of an improvement that I think we should take this patch even if it doesn't fix the "loads the wrong URLs" part right now, as long as no one spots any regressions.)

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

OK, more info on at least one case of the "wrong page" problem (this is different from the paste-return operation I performed in comment 6, because that on was just pasting an entire URL, but the same fix might paper over that case, too):

I started typing "openradar" into my location bar, I arrowed down to get the location bar to fill in ".appspot.com/" and then placed the cursor in the location bar, pasted "4559968" and hit return.

Camino loaded the "openradar.appspot.com/" history item :-(  

This appears to be a race between hitting return and the autocomplete window discovering that whatever's pasted no longer matches what's been selected (and, indeed, if I wait several seconds, the autocomplete window closes or loses selection.

To fix this case, it seems like we need to do one of two things:

When the user places the cursor in the location bar after doing something in the autocomplete window, either

a) make sure the location bar "handles" return rather than the the autocomplete window, or 

b) kill the "selection"/"focus" in the autocomplete window, so it doesn't have anything to act upon if it gets the return keyEvent, or

c) close the autocomplete window entirely.

I don't like b) or c) because they (unnecessarily) kill state information (e.g., I might have made a bad paste, or a bad selection in the window before the paste, and want to arrow up/down to select another "base URL" and start over).  

In addition, it seems like we're really more or less doing b) or c) on a delay, after the paste creates a non-matching condition (rather than on a "focus" change/click in the location bar), and I think to prevent the race, we really need to act ASAP and tell the autocomplete window to stop responding to return after the click event in the location bar (of course, when arrowing down again after having pasted, we want to reset the window's ability to respond).

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

That might fix the comment 6 full-paste scenario, or any case where there's a clear change-of-focus-to-the-location-bar, but it might not fix the type-then-paste scenario, since we're already in/editing in the location bar; we'd have to listen for all sorts of different editing/cursor events and decide which ones should make us tell the autocomplete window to stop handling return.  Hrm.

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 437480 [details] [diff] [review]
Fix v1.0

I did a bunch of URL editing tonight, and I haven't seen any side effects or other issues.

For me, then, the net effect of this patch is to prevent the autocomplete window from popping up after autocompleting.  That's certainly a plus (and worth landing, IMO), even if it doesn't fix the "sending you to the wrong URL" cases.

Comment 10

[Stuart Morgan]

Comment on attachment 437480 [details] [diff] [review]
Fix v1.0

r=me

Comment 11

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Bug 561695 sounds like someone seeing the "reappearing after autocompleting" behavior on 1.9.0, too.

Comment 12

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 437480 [details] [diff] [review]
Fix v1.0

sr=pink

Comment 13

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

http://hg.mozilla.org/camino/rev/8c71ff40682f and cvs trunk.

I spun the remaining stuff (editing/sending you to wrong URL) into bug 563094.