Closed
Bug 844716
Opened 12 years ago
Closed 12 years ago
Enable keyboard Apps to get/set selection range of the input field
Categories
(Firefox OS Graveyard :: General, defect, P2)
Firefox OS Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: xyuan, Assigned: xyuan)
References
()
Details
Attachments
(3 files, 2 obsolete files)
The virtual keyboard API need to be extend to allow keyboard app to get or set the selection range of the input field. That feature is required by at least the following two cases:
# Swipe selection. It is to move the cursor or selection range by swiping on the keyboard, witch needs to both get and set the selection range of the input field.
# Word suggestion or correction, witch needs to get the cursor position of the input field.
Assignee | ||
Comment 1•12 years ago
|
||
The functionality of getting selection range depends on BUG 818893.
Depends on: 818893
Assignee | ||
Comment 2•12 years ago
|
||
The portion of the extended keyboard API(https://wiki.mozilla.org/WebAPI/KeboardIME) related to setting/getting selection range of the input field would be implemented for this bug:
// Fires when user moves the cursor, changes the selection, or alters
// the composing text length
attribute Function onselectionchange;
// The start and stop position of the selection.
readonly attribute long selectionStart;
readonly attribute long selectionEnd;
// Set the selection range of the the editable text.
// Note that the start position should be less or equal to the end position.
// To move the cursor, set the start and end position to the same value.
void setSelectionRange(in long start, in long end);
The above methods and properties can be accessed from navigator.mozKeyboard.
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #719407 -
Flags: review?(dflanagan)
Comment 4•12 years ago
|
||
I haven't looked at this code in a long time, and have not been paying attention to the plans for the new keyboard API, so I am not the best reviewer for this. I will try to review this patch next week. In the meantime, however, feel free to replace me with someone who knows more about it.
Comment 5•12 years ago
|
||
Comment on attachment 719407 [details] [diff] [review]
Keyboard APIs regarding selection range
Tim, can you review (or re-assign to the right person?)
Attachment #719407 -
Flags: review?(dflanagan) → review?(timdream)
Comment 6•12 years ago
|
||
Comment on attachment 719407 [details] [diff] [review]
Keyboard APIs regarding selection range
Looks alright if I have the right to review this code.
Attachment #719407 -
Flags: review?(timdream) → review+
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #6)
> Comment on attachment 719407 [details] [diff] [review]
> Keyboard APIs regarding selection range
>
> Looks alright if I have the right to review this code.
@Tim, thanks. I'll make a little change to rebase the patch.
Flags: needinfo?
Assignee | ||
Comment 8•12 years ago
|
||
1. Rebase
2. Check the element parameter of the setSelectionRange function in the file forms.js to prevent setting the selection range of a selector:
function setSelectionRange(element, start, end) {
let isPainTextField = element instanceof HTMLInputElement ||
element instanceof HTMLTextAreaElement;
// Check the parameters
if (!isPainTextField && !isContentEditable(element)) {
// Skip HTMLOptionElement and HTMLSelectElement elements, as they don't
// support the operation of setSelectionRange
return;
}
...
Attachment #719407 -
Attachment is obsolete: true
Attachment #724302 -
Flags: review?(timdream)
Flags: needinfo?
Comment 9•12 years ago
|
||
Comment on attachment 724302 [details] [diff] [review]
Keyboard APIs regarding selection range
s/isPainTextField/isPlainTextField/g (ouch, what a pain)
Attachment #724302 -
Flags: review?(timdream) → review+
Updated•12 years ago
|
Assignee: nobody → xyuan
Component: Gaia::Keyboard → General
Assignee | ||
Comment 10•12 years ago
|
||
Fix typos in b2g/chrome/content/forms.js with the following replacement:
s/isPainTextField/isPlainTextField/g
Attachment #724302 -
Attachment is obsolete: true
Attachment #724942 -
Flags: review?(timdream)
Comment 11•12 years ago
|
||
Comment on attachment 724942 [details] [diff] [review]
Keyboard APIs regarding selection range
Review of attachment 724942 [details] [diff] [review]:
-----------------------------------------------------------------
Just carry over the r+ :)
Attachment #724942 -
Flags: review?(timdream) → review+
Assignee | ||
Comment 12•12 years ago
|
||
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (PTO Mar 22 - Apr 7) from comment #11)
> Comment on attachment 724942 [details] [diff] [review]
> Keyboard APIs regarding selection range
>
> Review of attachment 724942 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Just carry over the r+ :)
Thank you ;-)
So it's time to check in?
Status: NEW → ASSIGNED
Keywords: checkin-needed
Comment 13•12 years ago
|
||
Keywords: checkin-needed
Comment 14•12 years ago
|
||
Does that mean that it will be included in the next B2G build?
Comment 15•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
QA Contact: wachen → whsu
Assignee | ||
Comment 16•12 years ago
|
||
Reopen the bug as onselectionchange needs a patch.
If cursor position is changed by JS, for example, clearing the text field with js and changing the cursor position to 0(See bug 860546 for details), we cannot get selectionchange event from the listen assigned to onselectionchange attribute.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 17•12 years ago
|
||
We are trying to squeeze cursor swiping into 1.1 (Leo), but it is depending on this bug. Do you have any estimates for when 844716 will be fixed and can be landed on trunk?
Flags: needinfo?(xyuan)
Assignee | ||
Comment 18•12 years ago
|
||
It'll be fixed in two or three weeks before May 15th and a slight change will be made to the API as well.
Flags: needinfo?(xyuan)
Comment 20•12 years ago
|
||
Has been implemented in mozilla-central. https://hg.mozilla.org/mozilla-central/file/8f9ba85eb61c/b2g/components/MozKeyboard.js#l119
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 21•12 years ago
|
||
It has been implemented in mozilla-central. But the issue mentioned in comment 16 still exits.
We need create a new bug to track the issue.
Flags: needinfo?(xyuan)
Comment 22•12 years ago
|
||
Just did a quick verification on Mozilla-central with latest build.(Mozilla-central-unagi-eng/2013-06-19-03-12-22).
I can select a range of the input field while I swipe the wording. As attachment.
But, I have one thing need to confirm. Did we only apply this feature on URL field?
I still cannot select a specific range of text on text field.
Comment 23•12 years ago
|
||
Assignee | ||
Comment 24•12 years ago
|
||
It should work for all types of text fields as well as contentEditable. You can check the Keyboard page of UITest app for a full list of text fields.
How did you test the selecting function and could you show me the steps?
Comment 25•12 years ago
|
||
I launched the Keyboard page of UITest app and tap the text field.
After I typed some characters on it, I swipe these characters.
I still cannot select part of characters.
As attachment.
Comment 26•12 years ago
|
||
Assignee | ||
Comment 27•12 years ago
|
||
William, than you.
The patch enables keyboard(such as the Latin keyboard) to manipulate the selection range of the input field, but doesn't allow user to select a specified range of text by swiping the text field.
To test the patch, we may probably need to create a new type of keyboard with a 'test' button. Switch to that keyboard and click the test button to call navigator.mozKeyboard.setSelectionRange to change the selected text.
You need to log in
before you can comment on or make changes to this bug.
Description
•