Closed Bug 1082425 Opened 5 years ago Closed 4 years ago

Composition string is removed when selection caret mode by long tap

Categories

(Core :: DOM: Selection, defect, P4)

ARM
Gonk (Firefox OS)
defect

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: m_kato, Assigned: TYLin)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

When moving to selection caret mode by long tap, composition string by B2G keyboard such as pin-yin is removed.

I think that this behavior isn't good.  We should commits composition string by IME when moving to selection mode. (or same as Android behavior)

Also, iOS and Android isn't move to selection mode when text has composition string.


- Step
1. set selectioncaret.enable = true by prefs.js
2. Change keyboard to Pin-Yin (or any keyboard that supports IME composition string)
3. Input any character in input box.  (ex. Tap [p] key 4 times).  Composition string is "pppp"
4. long tap for moving to selection mode.

- Result
composition string is removed, then moving to selection mode

- Expected Result
composition string is committed, then moving to selection mode.

or 

Don't move to selection mode when text has composition string.
I think we need discuss with UX about which result is what we want. So ni omega for the UX spec about this composition string behavior.
Flags: needinfo?(ofeng)
Committing composition is better for compatibility with desktop Gecko.

Anyway, b2g's forms.js should support nsIWidget::NotifyIME(REQUEST_TO_COMMIT_COMPOSITION) and nsIWidget::NotifyIME(REQUEST_TO_CANCEL_COMPOSITION) with moz-event or notification observer.
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #2)
> Committing composition is better for compatibility with desktop Gecko.
> 
> Anyway, b2g's forms.js should support
> nsIWidget::NotifyIME(REQUEST_TO_COMMIT_COMPOSITION) and
> nsIWidget::NotifyIME(REQUEST_TO_CANCEL_COMPOSITION) with moz-event or
> notification observer.

If UX team decides that this behavior is same as Android/iOS's way, it is unnecessary to fix bug 917323.
(In reply to Makoto Kato (:m_kato) from comment #3)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #2)
> > Committing composition is better for compatibility with desktop Gecko.
> > 
> > Anyway, b2g's forms.js should support
> > nsIWidget::NotifyIME(REQUEST_TO_COMMIT_COMPOSITION) and
> > nsIWidget::NotifyIME(REQUEST_TO_CANCEL_COMPOSITION) with moz-event or
> > notification observer.
> 
> If UX team decides that this behavior is same as Android/iOS's way, it is
> unnecessary to fix bug 917323.

I don't think so. I believe that there are other bugs which are caused by ignoring the request from content.

Even if IME on B2G always cancels composition, it should be triggered by REQUEST_TO_COMMIT_COMPOSITION.
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #4)
> (In reply to Makoto Kato (:m_kato) from comment #3)
> > (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #2)
> > > Committing composition is better for compatibility with desktop Gecko.
> > > 
> > > Anyway, b2g's forms.js should support
> > > nsIWidget::NotifyIME(REQUEST_TO_COMMIT_COMPOSITION) and
> > > nsIWidget::NotifyIME(REQUEST_TO_CANCEL_COMPOSITION) with moz-event or
> > > notification observer.
> > 
> > If UX team decides that this behavior is same as Android/iOS's way, it is
> > unnecessary to fix bug 917323.
> 
> I don't think so. I believe that there are other bugs which are caused by
> ignoring the request from content.
> 
> Even if IME on B2G always cancels composition, it should be triggered by
> REQUEST_TO_COMMIT_COMPOSITION.

That commnet is for this issue only.  As long plan, we need fix bug 917323.
Since the composition is not finished yet, it makes more sense to remove the composition string, and then switch to selection mode.
Flags: needinfo?(ofeng)
(In reply to Omega Feng [:Omega] [:馮於懋] (please ni?) from comment #6)
> Since the composition is not finished yet, it makes more sense to remove the
> composition string, and then switch to selection mode.

Maybe, you don't know usages of IME at others coutries.

Japanese IME and users doesn't aware to commited of composition string.  When inputing Kanji by IME, uses usually use the following.

1. tap hiragana words
2. conversion (by conversion button) hiragana to Kanji without commited if needed.
3. loop 1-2.

When step 3, IME commits composition string automatically.  Users don't aware to commit string.  IME will commit it automately.

But When using your idea, composition string is removed.  It isn't matched for Japanese usages and other OSes.

Android and iOS don't change over to selection mode when text has composition string because they know IME usages.
Hi Makoto, thanks for your explanation about Japanese IME usage, it's quite similar to Chinese IME usage actually. :)

Now we have some options below, and I add some comments inline:
1) Keep the composition string and don't switch to selection mode (iOS Chinese/Japanese)
   -> User may not know why he cannot select text under that condition, and it causes bad experience.
2) Switch to selection mode and bring the composition string to the selected position (Android Chinese)
   -> It doesn't make sense to type a string and then select where to insert it.
3) Commit the composition string and switch to selection mode (Android Japanese)
   -> The composition may not finished yet (e.g. user types な but actually he wants to type 奈良), so committing the incomplete composition will bother the user a lot.
4) Remove the composition string and switch to selection mode (My suggestion)
   -> Only the last composition is removed, not the whole sentence. Actually user can press Enter to commit the last composition to avoid this problem.

Basically 3) and 4) are OK, but I prefer 4) more. BTW, 4) has consistent behavior with the conclusion in bug 1062010.
(In reply to Omega Feng [:Omega] [:馮於懋] (please ni?) from comment #8)
> Hi Makoto, thanks for your explanation about Japanese IME usage, it's quite
> similar to Chinese IME usage actually. :)
> 
> Now we have some options below, and I add some comments inline:
> 1) Keep the composition string and don't switch to selection mode (iOS
> Chinese/Japanese)
>    -> User may not know why he cannot select text under that condition, and
> it causes bad experience.
> 2) Switch to selection mode and bring the composition string to the selected
> position (Android Chinese)
>    -> It doesn't make sense to type a string and then select where to insert
> it.
> 3) Commit the composition string and switch to selection mode (Android
> Japanese)
>    -> The composition may not finished yet (e.g. user types な but actually
> he wants to type 奈良), so committing the incomplete composition will bother
> the user a lot.
> 4) Remove the composition string and switch to selection mode (My suggestion)
>    -> Only the last composition is removed, not the whole sentence. Actually
> user can press Enter to commit the last composition to avoid this problem.
> 
> Basically 3) and 4) are OK, but I prefer 4) more. BTW, 4) has consistent
> behavior with the conclusion in bug 1062010.

Omega, thank you for considering our case!.  3) is better way for Japanese users.  About bug 1062010 case, I think that it is an edge case and IME will be able to handle whether commit or cancel on bug 1062010's situation.  But about selection, IME cannot receive any information/observers such as mozbrowserselectionchange.

To implement 3), we need implement NotifyIME like Nakano-san's comment #4 at first. I will file a bug for REQUEST_TO_COMMIT_COMPOSITION.
Priority: -- → P4
Blocks: AccessibleCaret
No longer blocks: CopyPasteLegacy
This also prevents that long tapping to select word while composing
corrupts the editable data on B2G.
This also prevents that long tapping to select word while composing
corrupts the editable data on B2G.
Attachment #8691912 - Attachment is obsolete: true
Comment on attachment 8691916 [details] [diff] [review]
Commit composition string before changing focus by long tap. (v2)

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

Mark, would you help test if this patch fixed bug 1224884?
Attachment #8691916 - Flags: review?(masayuki)
Attachment #8691916 - Flags: feedback?(markcapella)
Assignee: nobody → tlin
Comment on attachment 8691916 [details] [diff] [review]
Commit composition string before changing focus by long tap. (v2)

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

Oh, I like that !
Attachment #8691916 - Flags: feedback?(markcapella) → feedback+
Duplicate of this bug: 1224884
Comment on attachment 8691916 [details] [diff] [review]
Commit composition string before changing focus by long tap. (v2)

When user does long-tap in focused editor (i.e., it has composition and press screen in different point in the editor), the composition is committed at old caret position or new caret position? If the former, this must be right fix.
Flags: needinfo?(tlin)
Re comment 15: 

The string will be committed at its original position, i.e. at the old caret position.
Flags: needinfo?(tlin)
Comment on attachment 8691916 [details] [diff] [review]
Commit composition string before changing focus by long tap. (v2)

Thanks. Then, this makes the behavior as I expected.
Attachment #8691916 - Flags: review?(masayuki) → review+
https://hg.mozilla.org/mozilla-central/rev/3c0f581dc05b
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.