244900 - autocomplete does not set focus before mousemove event

Status: RESOLVED FIXED

(SeaMonkey :: Location Bar, defect)

Description

[Kristian Kræmmer Nielsen]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040206 Firefox/0.8
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040206 Firefox/0.8

The selection in the autocomplete selection component appearing while typing in
the location bar is not setting current focus if the mouse is not moved.

If you do not move the mouse and click on the autocompleted item you want to use
it is instead interbitated as "enter" or "go".


Reproducible: Always
Steps to Reproduce:
1. Place the cursor just below the location bar
2. Type anything that interduces the autocomplete-selection bar
3. Click the left ("action") mouse button.

Actual Results:  
The browser navigates the to incomplete URL typed instead of the autocompleted
URL actually clicked upon/selected.

Expected Results:  
When the autocomplete bar appear it should look at the location on the mouse to
determind initial selection.

This is not a problem with the native selection component, ie. used in Internet
Explorer

Comment 1

[Sam F.]

I have replicated this bug (244900) in Win XP Pro: Mozilla/5.0 (Windows; U;
Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0.

Comment 2

[Wayne Mery (:wsmwk)]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050307 

Confirm

Comment 3

[Andrew Schultz]

Attachment: patch

Comment 4

[Andrew Schultz]

*** Bug 168623 has been marked as a duplicate of this bug. ***

Comment 5

[[e-mail]]

Attachment: More extensive patch including some code simplification

Comment 6

[Ian Neal]

Comment on attachment 205621 [details] [diff] [review]
More extensive patch including some code simplification

>Index: autocomplete.xml
>===================================================================
.
.
>     <handlers>
>       <handler event="mouseout" action="this.textbox.view.selectedIndex = null;"/>
>-      <handler event="mouseup" action="this.textbox.onResultClick();"/>
>+      <handler event="mouseup"><![CDATA[
>+        this.textbox.view.selectedIndex = this.getHoverRow(event);
>+        this.textbox.onResultClick();
>+      ]]></handler>
Don't we need to test for (this.textbox.resultsPopup.selectedIndex == null) like in the original patch or this always the case?

Comment 7

[Andrew Schultz]

*** Bug 192925 has been marked as a duplicate of this bug. ***

Comment 8

[Andrew Schultz]

Comment on attachment 205621 [details] [diff] [review]
More extensive patch including some code simplification

With the patch applied, I get:

JavaScript error: , line 0: uncaught exception: [Exception... "Not enough arguments [nsITreeBoxObject.getCellAt]"  nsresult: "0x80570001 (NS_ERROR_XPC_NOT_ENOUGH_ARGS)"  location: "JS frame :: chrome://global/content/autocomplete.xml :: getHoverRow :: line 1595"  data: no]

Comment 9

[Andrew Schultz]

*** Bug 193626 has been marked as a duplicate of this bug. ***

Comment 10

[Andrew Schultz]

> JavaScript error: , line 0: uncaught exception: [Exception... "Not enough
> arguments [nsITreeBoxObject.getCellAt]"  nsresult: "0x80570001

+           var row = this.textbox.view.treeBoxObject.getCellAt(x, y);
should be getRowAt(x, y);

Comment 11

[[e-mail]]

(In reply to comment #6)
>>+      <handler event="mouseup"><![CDATA[
>>+        this.textbox.view.selectedIndex = this.getHoverRow(event);
>>+        this.textbox.onResultClick();
>>+      ]]></handler>
>Don't we need to test for (this.textbox.resultsPopup.selectedIndex == null)
>like in the original patch or this always the case?
I guess I should only call onResultClick if the index is not null.

(In reply to comment #10)
>>+           var row = this.textbox.view.treeBoxObject.getCellAt(x, y);
>should be getRowAt(x, y);
Doh :-[

Want a new patch?

Comment 12

[Ian Neal]

(In reply to comment #11)
> (In reply to comment #6)
> >>+      <handler event="mouseup"><![CDATA[
> >>+        this.textbox.view.selectedIndex = this.getHoverRow(event);
> >>+        this.textbox.onResultClick();
> >>+      ]]></handler>
> >Don't we need to test for (this.textbox.resultsPopup.selectedIndex == null)
> >like in the original patch or this always the case?
> I guess I should only call onResultClick if the index is not null.
> 
> (In reply to comment #10)
> >>+           var row = this.textbox.view.treeBoxObject.getCellAt(x, y);
> >should be getRowAt(x, y);
> Doh :-[
> 
> Want a new patch?
> 

Probably best.

Comment 13

[[e-mail]]

Attachment: Revised patch

Comment 14

[jag (Peter Annema)]

Comment on attachment 206271 [details] [diff] [review]
Revised patch

>Index: autocomplete.xml
>===================================================================
>+      <method name="getHoverRow">
...
>+           var row = this.textbox.view.treeBoxObject.getRowAt(x, y);
>+           if (row >= 0)
>+             return row;
>            else
>+             return null;

nit: return after else

sr=jag

Comment 15

[Ian Neal]

Comment on attachment 206271 [details] [diff] [review]
Revised patch

r=me with jags nit fixed :-)

Comment 16

[[e-mail]]

Fix checked in to the trunk.

Comment 17

[Steffen Wilberg]

Note that bug 306538 fixed this problem in toolkit's autocomplete.xml by adding a mousedown handler:
https://bugzilla.mozilla.org/attachment.cgi?id=194715&action=diff

Comment 18

[[e-mail]]

(In reply to comment #17)
>Note that bug 306538 fixed this problem in toolkit's autocomplete.xml by adding
>a mousedown handler:
OK, so who wants to come up with a list of (dis)advantages for down, up or both?