Closed Bug 851072 Opened 7 years ago Closed 5 years ago

Add cursor-swiping functionality to the Keyboard

Categories

(Firefox OS Graveyard :: Gaia::Keyboard, enhancement)

x86
macOS
enhancement
Not set

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: sergi, Unassigned)

References

()

Details

Attachments

(1 file, 2 obsolete files)

Implement cursor-swiping like in http://sergimansilla.com/blog/swipeselection-in-an-afternoon using the new Virtual Keyboard/IME API (https://wiki.mozilla.org/WebAPI/KeboardIME).
This is currently blocked by 844716. You can take a look at the current implementation of it here: https://github.com/comoyo/gaia/compare/master...swipe_noselection?w=1
Assignee: nobody → sergi.mansilla
Attached patch Github Pull Request (obsolete) — Splinter Review
Attachment #746349 - Flags: review?(timdream)
Comment on attachment 746349 [details] [diff] [review]
Github Pull Request

Redirect review. Overall the code looks OK.

Based on the discussion on dev-gaia I tend to think UX have already sign-off the feature? If not, (or you are unsure of), would you record a video and set feedback? to UX here?

The other thing I am unsure of would be whether or not it make sense to have a switch in the code (or even mozSettings) to turn the feature off (for customizations). Maybe a |var ENABLE_SWIPE = true| would do for now.
Attachment #746349 - Flags: review?(timdream)
Attachment #746349 - Flags: review?(rlu)
Attachment #746349 - Flags: feedback+
Comment on attachment 746349 [details] [diff] [review]
Github Pull Request

This issue depends on Bug 844716, which is only fixed on m-c, not on b2g.
Do you know whether we could check in this Gaia master with the assumption that Gaia master will be used with m-c?

BTW, this behavior change will cause that we can no longer select any other key on the same row by swiping, e.g. pressing "k" first, and then move your finger to "h".
This part definitely need UX input to confirm this is what we want.

Thank you.

--
I'll clear the review request first.
Will get back to it after the above questions are clear.
Attachment #746349 - Flags: review?(rlu)
Attachment #746349 - Flags: feedback?(fdjabri)
Attachment #746349 - Flags: feedback?
Attachment #746349 - Flags: feedback+
(In reply to Rudy Lu [:rudyl] from comment #4)
> This issue depends on Bug 844716, which is only fixed on m-c, not on b2g.
> Do you know whether we could check in this Gaia master with the assumption
> that Gaia master will be used with m-c?
> 

You should be able to probe for the the function, e.g. |if (mozKeyboard.setSelectStart())|

> BTW, this behavior change will cause that we can no longer select any other
> key on the same row by swiping, e.g. pressing "k" first, and then move your
> finger to "h".
> This part definitely need UX input to confirm this is what we want.

Yeah, UX need to confirm this. Please update a video.
> BTW, this behavior change will cause that we can no longer select any other
> key on the same row by swiping, e.g. pressing "k" first, and then move your
> finger to "h".
> This part definitely need UX input to confirm this is what we want.

Only in case the other key is not surrounding the key that the user is pressing, which I would say is 90% of the cases when the user wants to correct a mistyping. But yes, by all means UX has the last word on it.
Attachment #754793 - Flags: review?(francisco.jordano)
Attachment #754793 - Attachment is obsolete: true
Attached patch New Github PRSplinter Review
Added some suggestions by the reviewers.
Attachment #746349 - Attachment is obsolete: true
Attachment #746349 - Flags: feedback?(fdjabri)
Attachment #746349 - Flags: feedback?
Attachment #770093 - Flags: review?(timdream)
Attachment #770093 - Flags: feedback?(janjongboom)
Comment on attachment 770093 [details] [diff] [review]
New Github PR

Re-assigning r? because Tim is no longer keyboard owner.
Attachment #770093 - Flags: review?(timdream) → review?(rlu)
Tested on GP Keon, Gecko 821f644.

* Initial scroll goes well, cursor is in right place, when doing it again the cursor jumps to EOL (if right to left) or BOL (left to right) and then scrolls.
* Due to this scroll left to right to move from BOL to middle, then scroll right to left doesn't work, jumps back to BOL.
Attachment #770093 - Flags: feedback?(janjongboom) → feedback-
I dont know whether this is caused by this bug but when I click the address bar in the browser it no longer replaces the title by the URL and the text isnt selected either like before.
Depends on: 889347
Updated PR addressing positioning bugs, and now taking advantage of `selectionchange` bug, since it has been fixed in 889347.
Go into app with autocorrect. Type 'Jan is', swipe to the left to the 'n'. Suggestions don't change and internal keyboard state is f* up, f.e. tap 'ISP', text changes to 'JISP'.
I have not seen any UX sign off here. Has it been given somewhere in a mailing list?
Flags: needinfo?(jcarpenter)
Attachment #770093 - Flags: review?(rlu)
Attachment #770093 - Flags: review?(janjongboom)
Attachment #770093 - Flags: feedback-
Looks like swiping interferes with dictionary word suggestion in textboxes like the one in the SMS app. This is most likely because the suggestion mechanism is triggered every time the cursor position changes, and a fast swiping causes the whole app to choke.

Not sure if that would be a keyboard problem or the suggestion mechanism should add checks to not be triggered many times.

Thoughts?
(In reply to Sergi Mansilla from comment #15)
> Looks like swiping interferes with dictionary word suggestion in textboxes
> like the one in the SMS app. This is most likely because the suggestion
> mechanism is triggered every time the cursor position changes, and a fast
> swiping causes the whole app to choke.
> 
> Not sure if that would be a keyboard problem or the suggestion mechanism
> should add checks to not be triggered many times.
> 
> Thoughts?

It would be very easy for the suggestion mechanism to throttle look-ups and sounds like a pretty sensible thing to do anyway - could this end up being a possible DoS vector, for example?
Flags: needinfo?(janjongboom)
Recently there's a support for 3rd party keyboard in gaia master.

Have you consider make cursor positioning/select (or even copy/paste) in a brand new 3rd party keyboard (with minimum keys), instead of integrate in the default one? 


(For reference there's a blank 3rd-party keyboard demo app in gaia/test_apps/test-keyboard-app)
Based on the plans we have for 1.3+ keyboard support we'll come with gestures on the keyboard ourselves that will overlap here.
Flags: needinfo?(janjongboom)
(In reply to Jan Jongboom [:janjongboom] from comment #18)
> Based on the plans we have for 1.3+ keyboard support we'll come with
> gestures on the keyboard ourselves that will overlap here.

What kind of plan are you referring to? 
Can you share more details?
(In reply to Rudy Lu [:rudyl] from comment #19)
> (In reply to Jan Jongboom [:janjongboom] from comment #18)
> > Based on the plans we have for 1.3+ keyboard support we'll come with
> > gestures on the keyboard ourselves that will overlap here.
> 
> What kind of plan are you referring to? 
> Can you share more details?

https://bug908493.bugzilla.mozilla.org/attachment.cgi?id=797299 page 26.
Attachment #770093 - Flags: review?(janjongboom)
Duplicate of this bug: 939446
Assignee: sergi.mansilla → nobody
Although this is a nice-to-have feature, this may have been superseded by our current cursor movement work, bug 921964, which has been enabled on current master.

Sergi, are you still working on this?
If not, do you agree we could close this in favor of cursor movement design for now?
Flags: needinfo?(jcarpenter) → needinfo?(sergi.mansilla)
Rudy, definitely. I am not working on this anymore and I'm fine with closing it to focus on the new cursor movement design.
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(sergi.mansilla)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.