844716 - Enable keyboard Apps to get/set selection range of the input field

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect, P2)

Description

[Yuan Xulei [:yxl] (Leave from May 18)]

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.

Comment 1

[Yuan Xulei [:yxl] (Leave from May 18)]

The functionality of getting selection range depends on BUG 818893.

Comment 2

[Yuan Xulei [:yxl] (Leave from May 18)]

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.

Comment 3

[Yuan Xulei [:yxl] (Leave from May 18)]

Attachment: Keyboard APIs regarding selection range

Comment 4

[David Flanagan [:djf]]

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

[Dietrich Ayala (:dietrich)]

Comment on attachment 719407 [details] [diff] [review]
Keyboard APIs regarding selection range

Tim, can you review (or re-assign to the right person?)

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 719407 [details] [diff] [review]
Keyboard APIs regarding selection range

Looks alright if I have the right to review this code.

Comment 7

[Yuan Xulei [:yxl] (Leave from May 18)]

(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.

Comment 8

[Yuan Xulei [:yxl] (Leave from May 18)]

Attachment: Keyboard APIs regarding selection range

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;
   }
 ...

Comment 9

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 724302 [details] [diff] [review]
Keyboard APIs regarding selection range

s/isPainTextField/isPlainTextField/g (ouch, what a pain)

Comment 10

[Yuan Xulei [:yxl] (Leave from May 18)]

Attachment: Keyboard APIs regarding selection range

Fix typos in b2g/chrome/content/forms.js with the following replacement:
 s/isPainTextField/isPlainTextField/g

Comment 11

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 724942 [details] [diff] [review]
Keyboard APIs regarding selection range

Review of attachment 724942 [details] [diff] [review]:
-----------------------------------------------------------------

Just carry over the r+ :)

Comment 12

[Yuan Xulei [:yxl] (Leave from May 18)]

(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?

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/901a79e85148

Comment 14

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Does that mean that it will be included in the next B2G build?

Comment 15

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/901a79e85148

Comment 16

[Yuan Xulei [:yxl] (Leave from May 18)]

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.

Comment 17

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

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?

Comment 18

[Yuan Xulei [:yxl] (Leave from May 18)]

It'll be fixed in two or three weeks before May 15th and a slight change will be made to the API as well.

Comment 19

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Any news on this issue?

Comment 20

[Jan Jongboom [:janjongboom] (inactive)]

Has been implemented in mozilla-central. https://hg.mozilla.org/mozilla-central/file/8f9ba85eb61c/b2g/components/MozKeyboard.js#l119

Comment 21

[Yuan Xulei [:yxl] (Leave from May 18)]

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.

Comment 22

[William Hsu [:whsu]]

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

[William Hsu [:whsu]]

Attachment: Wording selection

Comment 24

[Yuan Xulei [:yxl] (Leave from May 18)]

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

[William Hsu [:whsu]]

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

[William Hsu [:whsu]]

Attachment: Cannot select characters

Comment 27

[Yuan Xulei [:yxl] (Leave from May 18)]

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.