606196 - Add a method to directly select the end of the text in nsIAutoCompleteInput.idl

Status: RESOLVED WONTFIX

(Toolkit :: Autocomplete, defect)

Description

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

Currently, to move the caret at the end of the input, a lot of code is doing:
input->SelectTextRange(value.Length(), value.Length());

But, value is the |value| from the autocomplete. So, with @list and @multiple, |value| isn't correct and the element's value has to be added. So, adding a new method to directly select the end of the text might be easier and more correct.

Comment 1

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

Comment on attachment 485042 [details] [diff] [review]
Patch v1

>diff --git a/toolkit/components/autocomplete/public/nsIAutoCompleteInput.idl 

> /* 
>+ * Put the caret at the end of the autocompleted text. 
>+ */ 
>+ void selectEndOfText();

I'd prefer something like "moveCaretToEnd()" - mentioning "select" is accurate, but confusing, since you're not really "selecting" text in the classic sense. 

>diff --git a/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp b/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp

It seems to me like nsAutoCompleteController::CompleteValue would be broken by this change. And just in general, it's not really expected by users of nsIAutoCompleteInput that getting textValue immediately after setting it would return some other value. I guess that may be a require change for bug 601209, but it should be documented on the interface at least.

Comment 2

[Mounir Lamouri (:mounir)]

(In reply to comment #1)
> Comment on attachment 485042 [details] [diff] [review]
> Patch v1
> 
> >diff --git a/toolkit/components/autocomplete/public/nsIAutoCompleteInput.idl 
> 
> > /* 
> >+ * Put the caret at the end of the autocompleted text. 
> >+ */ 
> >+ void selectEndOfText();
> 
> I'd prefer something like "moveCaretToEnd()" - mentioning "select" is accurate,
> but confusing, since you're not really "selecting" text in the classic sense. 

I agree.

> >diff --git a/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp b/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp
> 
> It seems to me like nsAutoCompleteController::CompleteValue would be broken by
> this change. And just in general, it's not really expected by users of
> nsIAutoCompleteInput that getting textValue immediately after setting it would
> return some other value. I guess that may be a require change for bug 601209,
> but it should be documented on the interface at least.

I do not use this new method in nsAutoCompleteController::CompleteValue so it should be fine, isn't it?
Actually, I'm doing some refactorization in bug 601209 and calls to SelectTextRange are reducing (if my patch doesn't change drastically since being pushed) so we can even WONTFIX this bug if you think the addition would be useless.

Comment 3

[Shawn Wilsher :sdwilsh]

Note that since this has an API change, we'll need to take it for beta 7 else slip it to Firefox next.

Comment 4

[Mounir Lamouri (:mounir)]

I did some refactoring in bug 601209 so this change might not be needed anymore.