1895643 - Cannot type phone number with ATOK as the order you type in the web casher form of Paypay

Status: NEW

(GeckoView :: IME, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves)]

STR:

1. Set the default keyboard/IME to ATOK Passport Pro in the system settings
2. Set "Open links in apps" setting of Firefox to not "Always" (I set it to "Ask before opening")
3. Go to some EC site which support Paypay (I use DLsite) and add something to the cart, then go advance in the cart to pay
4. Type the phone number (which is user ID of Paypay) as "0901234"

AR:
The input field becomes "090-2341"

ER:
The input field should be "090-1234"

After inserting the hyphen by Paypay, caret appears after the hyphen but before 1. Therefore, following input are inserted after the hyphen instead of after 1.

I cannot reproduce this bug with GBoard nor Samsung keyboard. So this is ATOK specific. I couldn't check the behavior on Chrome for Android because Chrome opens the Paypay app.

If I type something in <textarea> like here and tap non-composing text during composition to commit composition and move caret to tapped position, next typing will be inserted after the committed composition string. So I guess that ATOK always set start position of new composition. I guess that we notify selection change before commit request?

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves)]

In the worst scenario, we should not dispatch eSetSelection event if specified offset is same as the end offset of the last composition.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves)]

Although I'm not sure these things are directly related to this bug, I feel odd:

1. mIgnoreSelectionChange is set to true if the text change does not change the Gecko side text and it's already true or the last action is a change of text. However, selection change is different feature from text change. Therefore, ignoring following selection change must be wrong because the following selection change may cause moving selection not related to the changing text range.
2. mIgnoreSelectionChange is not set to false if the text change changes part of the text. Therefore, if it's already set to true, following selection change is always ignored, but I guess that this is not a problem with current IME notification design.
3. causedByComposition for selection change is considered with same name state of pending text change notification. This is completely odd. It's possible case that text change is caused by IME composition, but only selection change is caused by user interaction.

Comment 3

[Makoto Kato [:m_kato]]

Actually, BaseInputConnection's selection handling doesn't match with Gecko's way. So we have to sync it if possible. So it means that we have to set selection more times per input.

But, if ATOK uses InputConnection.commitText to input number, bug 1888330 may be able to help this issue...

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves)]

Attachment: Minimized testcase

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves)]

(In reply to Makoto Kato [:m_kato] (PTO until 5/6) from comment #3)

Actually, BaseInputConnection's selection handling doesn't match with Gecko's way. So we have to sync it if possible.

Yeah, I understand.

So it means that we have to set selection more times per input.

Well, my point here is, ATOK may not receive selection change notification which should be sent after committing composition in content and that could cause this bug. The minimized testcase indicates that selection update occurs after setting the <input>.value which should've caused requesting IME to commit composition so the new composition should be started at new caret position which is set by Paypay rather than the offset where the old composition string ends.

Then, I checked the Android widget code (including to learn what it does). Then, I found some logically wrong points listed in comment 2. Especially its 3rd one could cause this bug because it may fail notifying IME of selection change.

But, if ATOK uses InputConnection.commitText to input number, bug 1888330 may be able to help this issue...

I don't check which API is used by ATOK, sorry.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves)]

With the minimized testcase, I reproduce this bug with both GBoard and ATOK, but they work on Chrome.

Comment 7

[Makoto Kato [:m_kato]]

When calling InputConnection.setComposingText("1234", 1) on IME, selection position is mismatched with expected. GBoard (English layout) seems to use InputConnection.commitText(). So this issue are on GBoard (Japanese layout / alphabet input) and ATOK as long as I test.

Comment 8

[Makoto Kato [:m_kato]]

Attached test case may be incorrect. input event is fired twice, then second call sets that selection is 4... So this reproduce on Linux too.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves)]

(In reply to Makoto Kato [:m_kato] from comment #8)

Attached test case may be incorrect. input event is fired twice, then second call sets that selection is 4... So this reproduce on Linux too.

Oh? I forgot about our input event issue at ending composition. However, I'm not sure about the last half.

At first input event, selStart should be 4 first. Then, I'll be incremented before setting selection range. Then, selection will be collapsed at 5. Then, at second input event, selStart is 5 at first. Then, it won't be incremented because of the previous char is the last input digit. Then, selection will be collapsed as-is. So, it should be 123-4[].

Comment 10

[Makoto Kato [:m_kato]]

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6) from comment #9)

(In reply to Makoto Kato [:m_kato] from comment #8)

Attached test case may be incorrect. input event is fired twice, then second call sets that selection is 4... So this reproduce on Linux too.

Oh? I forgot about our input event issue at ending composition. However, I'm not sure about the last half.

input.value = formattedValue; causes input event (insertCompositionText). Bug?

Comment 11

[Makoto Kato [:m_kato]]

Attachment: mochitest (sample)

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves)]

(In reply to Makoto Kato [:m_kato] from comment #10)

input.value = formattedValue; causes input event (insertCompositionText). Bug?

No, it should be fired because the composition is forcibly committed by setting the value. So, first one is caused by the user input, second one is caused by IME.

(In reply to Makoto Kato [:m_kato] from comment #8)

Attached test case may be incorrect. input event is fired twice, then second call sets that selection is 4... So this reproduce on Linux too.

Oh, indeed. I reproduced this with the testcase on Windows too. However, it works on Chrome on Windows. So, the component may be wrong (or I just failed to create the testcase).

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves)]

At the first setting value, here should request to commit composition. Then, PuppetWidget should dispatch eCompositionCommit event synchronously. Then, EditorBase::OnCompositionChange should insert the composition as-is. At this moment, the value should be 1234[]. Then, eCompositionEnd event should be fired by TextComposition and EditorBase::OnCompositionEnd should fire another input event. Okay, then, the script runs again and sets 123-4[] again and the value should becomes 123-4[]. Finally, TextControlState::SetValue restarts to set the first value, 123-4, and <input>.setSelectionRange is called for 123-4[]. That's the design of current code.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves)]

Hmm, unfortunately, fixing bug 1897865 couldn't fix this bug. I need to retry to create another testcase...

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves)]

The original source code is:

      function T(M) {
        const N = M.replace(/-/g, ''),
        u = N.substring(0, 3),
        D = N.substring(3, 7),
        T = N.substring(7, 11),
        j = [
          u,
          D,
          T
        ].filter((M => Boolean(M))).join('-');
        return j
      }

       function * () {
           let I = M.selectionStart ||
           0;
           const A = (0, j.un) (u),
           g = A[I - 1];
           N('update:modelValue', A),
           yield(0, T.Y3) (),
           A.length > D.length &&
           '-' === g &&
           I++,
           M.setSelectionRange(I, I)
         }

I think that this is the true testcase to reproduce the issue on in PayPay. The root cause must be same as bug 1897865, however, setting value occurs asynchronously in the input event handler. Therefore, the new hack landed in bug 1897865 does not work. I'll file a new blocker bug.

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves)]

Sigh... this is not fixed by the bug fix of bug 1904192. I have no more ideas from TextControlState and EditorBase side...