420804 - When autocomplete dropdown appears, entry below cursor is highlighted (interfering with arrow-key navigation)

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Daniel Holbert [:dholbert]]

This is a follow-up bug from Bug 408723 comment 77 and Bug 408723 comment 78.

Steps to reproduce:
 0. Clear history, or start with fresh profile. (for easy reproducibility)

 1. Visit these URLs:
     ftp://ftp.mozilla.org/pub
     ftp://ftp.mozilla.org/pub/artwork
     ftp://ftp.mozilla.org/pub/calendar
     ftp://ftp.mozilla.org/pub/camino/
     ftp://ftp.mozilla.org/pub/directory
     ftp://ftp.mozilla.org/pub/xulrunner

 2. Type "pub" into the location bar.  (This should bring up a list of the URLs given above)

 3. Position the cursor over a URL halfway down the list

 4. Press Ctrl-L to jump to highlight full location bar. Press delete to clear it.

 5. Press "p".
    *** RESULT: The autocomplete entry under the cursor is highlighted.  If you press uparrow/downarrow keys, they navigate relative to that highlighted entry.

 6. Press "u".
    *** RESULT: The highlighting goes away.  If you press uparrow/downarrow keys, they navigate relative to the position before the first entry (as desired)

 7. Press "z".  This makes the autocomplete panel disappear, as "puz" hasn't been seen before.

 8. Press "backspace".  This makes the autocomplete panel reappear.
    *** RESULT: Like after step 5, the entry under the cursor is highlighted!

Tested using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b4pre) Gecko/2008030307 Minefield/3.0b4pre

Comment 1

[Daniel Holbert [:dholbert]]

BTW -- in step 5 from comment 0, there's nothing special about the fact that we're only typing one letter.

If you're quick, you can get two or more characters entered before the autocomplete menu appears.  The bug still reproduces in that situation, with the entry under the cursor being highlighted **when the autocomplete menu appears**, regardless of how many characters you've typed at that point.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

This could be fixed by fixing either of bug 297080 or bug 417053.

Comment 3

[Sylvain Pasche]

Or it could be fixed by attachment 305372 [details] [diff] [review] (see bug 408723 comment 50)

Comment 4

[Sylvain Pasche]

Attachment: patch

This is the updated version of the patch that was in bug 408723.

If there is interest in going in that direction I can try writing a unit test.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #4)
> If there is interest in going in that direction I can try writing a unit test.

I'm not sure that we'd want to take a patch like that, assuming bug 297080 gets fixed. It's OK as a workaround, but I don't think it's the behavior we want in general.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Bug 297080 was landed, can someone confirm that this is now fixed?

Comment 7

[Daniel Holbert [:dholbert]]

I just tested a freshly checked out debug build, and it looks like this bug is only *partly* fixed.

From the original description in Comment #0:

>  5. Press "p".
>     *** RESULT: The autocomplete entry under the cursor is highlighted.  

This part is now fixed.

>  7. Press "z".  This makes the autocomplete panel disappear, as "puz" hasn't
> been seen before.
> 
>  8. Press "backspace".  This makes the autocomplete panel reappear.
>     *** RESULT: Like after step 5, the entry under the cursor is highlighted!

This part is NOT fixed.  When backspacing turns an unrecognized string into a recognized string, the entry under the cursor becomes highlighted/selected.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

That's odd. Is it still being selected because of a spurious mousemove event, or is it selected because it was somehow otherwise selected beforehand (i.e. similar to bug 406487)?

Comment 9

[Daniel Holbert [:dholbert]]

Pretty sure it's due to a spurious mousemove.

In between Steps 7 and 8 (when the dropdown list is gone), I can change the position of the mouse, and then when I complete step 8, the selected entry will be the one under the mouse's **new position**.  So it's not like this entry was preselected because I was hovering over it before, or something like that.

CAVEAT: I'm just noticing -- this remaining bug only seems to happen if I'm hovering over one of the **upper 5** history entries, but does *not* happen when I'm hovering over the last 4. (there are 9 total entries visible)

Comment 10

[Matthew Gregan [:kinetik]]

It is a spurious mouse mouse, caused by a mouse exit being converted into a mouse move in the ESM.

There's two things going on here.  When the dropdown appears for the first time, we get a single leave notify for the owning window, which is correctly dispatched into Gecko as a mouse exit marked as a top level exit.  For some reason, when the dropdown disappears and reappears like in step 8, we get two leave notify events, one exactly like the first, and the other targeted at the dropdown window, which we (correctly) treat as a child exit.  This second exit is converted to a mouse move in the ESM and causes the entry selection in the dropdown.  I'd expect to always see the entry under the pointer selected, not just when it's one of the first few entries.

The patch from bug #297080 can't handle the situation in which this second leave notify occurs.  I never ran into it during testing because of the second thing that's going on (see below), so I'm glad dholbert caught it.

The second thing that's going on is that during handling of the second (child) exit, either the view targeted by the event in nsView.cpp::HandleEvent or the frame targeted by the event in PresShell::HandleEvent is giving weird results in some cases.  I don't have a better explanation yet--the reason being that while I can consistently reproduce the behaviour where the later entries are not selected during normal use, I'm seeing them selected (as expected due to the first problem) when trying to debug.

Comment 12

[Matthew Gregan [:kinetik]]

I'm not sure what to do about this.  I can't think of a way to suppress the
second leave notify (or even convince myself that would be the correct
solution).  I toyed with the idea of dropping all child exits on the floor in
nsEventStateManager (since, in theory, mouse moves should be sufficient to
generate the DOM over/out events), which solves this and seems to behave
correctly in most cases but breaks important things like correct exit handling
when moving into plugin content (which perhaps we could solve another way, but
I have a feeling this rabbit hole is quite deep).

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

> the other targeted at the dropdown window, which we (correctly) treat as a
> child exit.

Why is that correct? Seems to me the dropdown window should be treated as toplevel, and that an exit from it should be treated as a toplevel exit.

Comment 15

[Matthew Gregan [:kinetik]]

(In reply to comment #13)
> Why is that correct? Seems to me the dropdown window should be treated as
> toplevel, and that an exit from it should be treated as a toplevel exit.

It is treated as a toplevel and we treat exits that are caused by entering it as toplevel exits.  The problem is that in the case where the dropdown reappears (step #8 in dholbert's testcase), we get a second leave notify which, it turns out, is caused by "movement" from the dropdown window into a child of the dropdown, so it's correct to treat it as a child exit...  except that we'd rather not see it at all, because it will be turned into a mouse move in the ESM.

I'm not sure why we only get the second leave when the dropdown reappears (maybe it's creation ordering differences).

So:
1. We can't just drop all child exits in the ESM due to problems with exiting into plugin content.
2. We can't filter this particular leave notify (like we do with certain types already) because we want it in other cases and there's no obvious way to detect this particular case.

I could try to work out why it only happens when the window reappears, and workaround that, but the problem might show up somewhere else then (but maybe that's okay for now (knowing Compositor is coming), if we fix this really visible and annoying case without breaking anything else).

Or I could look at making sure we exit into plugins differently so that we can drop child exits, assuming this is possible.

Alternatively, it might be possible to track the enter/exit state on toplevel windows directly, and just drop any enter/leave notifies that are already within that toplevel (but this might also have problems with exiting into plugins, and may also require walking a large chunk of our widget tree to determine what to do).

Or move towards what the Windows and Cocoa widget code does, which is synthesize mouse enter/exit events based on mouse movement (and also the MouseTrailer stuff on Windows).

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Can we suppress leave-notify when the mouse pointer hasn't actually moved?

Comment 21

[Rui Matos]

This bug is partially solved on FF3b5 (on linux at least). Apologies for the awful ASCII art :-)

         ----------------------------------------------------
         |*| awesome bar                                 |\/|
         ----------------------------------------------------
| tab | tab | tab | tab | tab | tab | tab | tab | tab | taXXX tab |
----------------------------------------------------------XXX------
|                                                         XXX     |
|                                                         XXX     |
|                                                         XXX     |
|                                                         XXX     |
-------------------------------------------------------------------

Currently if the mouse pointer is on the area marked with XXX when the awesome bar appears then the nearer entry to left comes up already selected. The XXX area is where the GTK+ scrollbar shows up.

Comment 22

[Matthew Gregan [:kinetik]]

Fixed by the patch for bug 428680.