Closed
Bug 606196
Opened 14 years ago
Closed 14 years ago
Add a method to directly select the end of the text in nsIAutoCompleteInput.idl
Categories
(Toolkit :: Autocomplete, defect)
Toolkit
Autocomplete
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: mounir, Assigned: mounir)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
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.
Attachment #485042 -
Flags: review?(gavin.sharp)
Assignee | ||
Updated•14 years ago
|
Attachment #485042 -
Attachment is patch: true
Attachment #485042 -
Attachment mime type: application/octet-stream → text/plain
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Comment 1•14 years ago
|
||
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.
Assignee | ||
Comment 2•14 years ago
|
||
(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•14 years ago
|
||
Note that since this has an API change, we'll need to take it for beta 7 else slip it to Firefox next.
Assignee | ||
Comment 4•14 years ago
|
||
I did some refactoring in bug 601209 so this change might not be needed anymore.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → WONTFIX
Assignee | ||
Updated•14 years ago
|
Attachment #485042 -
Flags: review?(gavin.sharp)
You need to log in
before you can comment on or make changes to this bug.
Description
•