203754 - Bug in select code in Auto-Complete component

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[eric lindvall]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4a) Gecko/20030423 Mozilla Firebird/0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4a) Gecko/20030423 Mozilla Firebird/0.6

The AutoCompleteController seems to have a bug in the Select code that seems to
be due to a change in how the input controller works (the selecting worked once).

it seems that mInput->SelectTextRange() doesn't like "-1" as the end offset anymore.

Reproducible: Always

Steps to Reproduce:
1. set completedefaultindex="true" in the urlbar in browser.xul
2. type something into the browser urlbar that doesn't start with "http://"
3. see results

Actual Results:  
none of the text is selected

Expected Results:  
the text that wasn't typed in should be selected.

I'll add the patch once i submit the bug.

Comment 1

[eric lindvall]

Attachment: Patch to fix problem

This seems to fix the problem.

Comment 2

[Erik]

I'll confirm this one and bug #203756 so the devs can have a look on it...

=> confirming and setting All/All

Comment 3

[David P James]

Taking QA. Sorry for the bugspam

Comment 4

[:deanis74]

Does this also fix the bug (at least I think it's a bug) where the cursor is
positioned at the start of the url bar instead of the end?

Comment 5

[eric lindvall]

no. i don't believe i've seen that behaviour.

Comment 6

[:deanis74]

That must be a different bug.  If you arrow-down through the items in the
suggested matches dropdown, the cursor moves to the start of the url bar.  It
used to, I'm sure, move to the end.  Maybe that was an intended change, but I
can't see why it would be.

Comment 7

[eric lindvall]

oh, yes, i believe this fixes that as well.

Comment 8

[eric lindvall]

Attachment: Improved patch to fix

This also fixes a problem where when using CompleteDefaultIndex, if you select
the end of a url and hit delete, the end of the url re-appears.

This now initializes mSearchString so the Backspaced check in HandleText() will
fire correctly.

Comment 9

[eric lindvall]

Comment on attachment 129372 [details] [diff] [review]
Improved patch to fix

please review.

Comment 10

[:deanis74]

Checked in.  Thanks Eric.

Comment 11

[David P James]

Hmmm... I can't verify this because I don't understand what the problem was to
begin with. I got lost at step 1 in comment #0 :(. Where would I set that?
chrome://browser/content/browser.xul ? But that doesn't make any sense to me.

Comment 12

[eric lindvall]

this did fix the problem.

the easiest way to turn this on would be to apply the patch in bug 203756 (which
adds a config option to turn "completedefaultindex" on and off).

Comment 13

[David P James]

Ok, I think I understand now - this was essentially an enabling bug to allow
another bug to be fixed. Fair enough.

verifying fixed as per reporter's confirmation; comment 12