Closed Bug 1518002 Opened 11 months ago Closed 11 months ago

Evented textarea jumping typing

Categories

(Firefox for Android :: Keyboards and IME, defect, P2)

Firefox 65
ARM
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 66
Tracking Status
firefox-esr60 --- unaffected
firefox64 --- unaffected
firefox65 + verified
firefox66 + verified

People

(Reporter: firefox, Assigned: m_kato)

References

Details

(Keywords: regression, Whiteboard: [geckoview])

Attachments

(2 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:65.0) Gecko/20100101 Firefox/65.0

Steps to reproduce:

Seen in version Firefox 65 on Android 6.0 (thanks Samsung). Attached video shows what happens in firefox with "bad" textarea, simple textarea, and with the "bad" textarea in samsung internet. Possibly related (but old) bugs: https://bugzilla.mozilla.org/show_bug.cgi?id=693266 https://bugzilla.mozilla.org/show_bug.cgi?id=1237286

1. Go to  a site that has a textarea that has event listeners on it (suspected cause)
2. Start typing
3. Try to change position and type again


Actual results:

Jumbled writing: text is written backwards, carat jumps first position if try to reposition and continue typing


Expected results:

Writing should work as normal (ltr, not jumping back and forth) as per a normal textarea / what happens in samsung internet
Screen capture video of bug: https://www.youtube.com/watch?v=8iZi7lGLwCY
Hi Jack,
I was not able to reproduce the issue. Tested on Samsung Galaxy Tab S3 (Android 8), on Firefox Beta 65.0b8 and Nightly 66.0a1 (2019-01-07).
Could you tell us the device you found the issue and the URL?
Thanks!
Flags: needinfo?(firefox)
Component: General → Text Selection
Hello. It is a Samsung Galaxy S5 Active. The textarea was on the issue editor form in gitlab.com (eg https://gitlab.com/bytesnz/ls-ignore/issues/new, but you need to log in to gitlab). I have seen it happen on a few other web sites, but can't remember which ones.
Flags: needinfo?(firefox)
Tested with Samsung Galaxy S6 (Android 7) on URL provided by you (made an account), but I could not reproduce the issue.
Do you have some add-ons installed? Could you try to test to see if you can reproduce with a clean profile?
Thank you!
Flags: needinfo?(firefox)

I tried it again on a fresh install of Firefox Beta on another phone (Samsung Galaxy S4 mini). It turns out I missed a step. Steps I used to reproduce:

  1. Goto website like https://gitlab.com/bytesnz/ls-ignore/issues/new
  2. Enter multiple lines of text into textare
  3. Click the Preview button above the textarea
  4. Click the Write button above the textarea to go back to the editor and textarea
  5. Focus somewhere in the textarea and start typing.
    Cursor starts jumping after a small pause

I noticed there is flashes of a scrollbar in/near the textarea, so I wonder if that has something to do with it. I tested it again in Samsung Internet and in Chrome and both did not show the issue.

Apologies for missing steps. Hopefully that will work for you.

Flags: needinfo?(firefox)

Hi Jack,
I was able to reproduce the issue, following the steps from Comment 5.
Tested with Samsung Galaxy S6 (Android 7) on the latest Nightly 66.0a1 (2019-01-09).
I will confirm it.
Thank you!

Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Unspecified → Android
Hardware: Unspecified → ARM
Attached file logcat

Please see attached logcat.

Ran a regression with Samsung Galaxy S7 Edge (Android 8.0):
Last good revision: 5cdef82c0fb01e17ab451c546f4bff1b90a1f7af
First bad revision: bda6c79cf03c921d16e4944fc200a424b4af78b0

Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=5cdef82c0fb01e17ab451c546f4bff1b90a1f7af&tochange=bda6c79cf03c921d16e4944fc200a424b4af78b0

Keywords: regression
Priority: -- → P2

Sounds like a bug of Android widget's IME handling code, but according to comment 8, this is a regression of bug 1465702... Odd. Why cannot we reproduce similar bug on desktop??

Anyway, bug 1465702 was clean-up of editor so that the changed points are too many. So, without minimized testcase which can reproduce this bug on desktop, it's really difficult to find the cause...

Flags: needinfo?(masayuki)

When I test this on Zenfone 5 (Android 8.0.0) with ATOK or GBoard, typing something after switching from Preview to Write, all text in <textarea> is removed.

Ccing Makoto-san, he might have some ideas...

The "Write" and "Preview" button handlers are here:

function(t) {
  var i = e(this);
  if (!n.allowAction(i))
    return n.stopEverything(t);
  var o = i.attr("name"),
    r = o ? {
      name: o,
      value: i.val()
    } : null,
    a = i.closest("form");
  0 === a.length && (a = e("#" + i.attr("form"))),
                     a.data("ujs:submit-button", r),
                     a.data("ujs:formnovalidate-button",
                     i.attr("formnovalidate")),
                     a.data("ujs:submit-button-formaction",
                     i.attr("formaction")),
                    a.data("ujs:submit-button-formmethod", i.attr("formmethod"))
}

and either one of the followings:

a() (document).on('markdown-preview:show', function (e, t) {
  t && (i = t.find('textarea.markdown-area'),
        d = i.height(),
        t.find('.js-md-write-button').parent().removeClass('active'),
        t.find('.js-md-preview-button').parent().addClass('active'),
        t.find('.md-write-holder').hide(),
        t.find('.md-preview-holder').show(),
        f.removeClass('active'),
        o.showPreview(t))
}),
a() (document).on('markdown-preview:hide', function (e, t) {
  t && (i = null, d && t.find('textarea.markdown-area').height(d),
        t.find('.js-md-write-button').parent().addClass('active'),
        t.find('.js-md-preview-button').parent().removeClass('active'),
        t.find('.md-write-holder').show(),
        t.find('textarea.markdown-area').focus(),
        t.find('.md-preview-holder').hide(),
        f.addClass('active'),
        o.hideReferencedCommands(t))
}),
Component: Text Selection → Keyboards and IME
Whiteboard: [geckoview]

I guess that DoReplaceText cannot handle this situation well.

01-15 16:56:16.995 24949 24965 D GeckoViewModule: dispatch GeckoView:ZoomToInput, data=null
01-15 16:56:16.996 24949 24965 D GeckoViewContent: onEvent: event=GeckoView:ZoomToInput, data=null
01-15 16:56:17.021 24949 24965 D GeckoViewModule: dispatch GeckoView:ZoomToInput, data=null
01-15 16:56:17.022 24949 24965 D GeckoViewContent: onEvent: event=GeckoView:ZoomToInput, data=null
01-15 16:56:17.027 24949 24965 D GeckoViewContent[C]: receiveMessage: GeckoView: ZoomToInput
01-15 16:56:17.029 24949 24965 D GeckoViewContent[C]: receiveMessage: GeckoView: ZoomToInput
01-15 16:56:17.117 24949 24965 I Gecko : nsWindow[0xc42b5380]::Resize [0.000000 48.000000 720.000000 526.000000] (repaint 0)
01-15 16:56:17.117 24949 24965 I Gecko : nsWindow: 0xc42b5380 OnSizeChanged [720 526]
01-15 16:56:17.134 24949 24949 D GeckoViewActivity: onFirstComposite
01-15 16:56:27.329 24949 25150 D GeckoInputConnection: > beginBatchEdit()
01-15 16:56:27.329 24949 25150 D GeckoInputConnection: < beginBatchEdit: true
01-15 16:56:27.331 24949 25150 D GeckoInputConnection: > setComposingText("G", 1)
01-15 16:56:27.332 24949 25150 D GeckoEditable: getSpanStart(android.view.inputmethod.ComposingText@832f942) = -1
01-15 16:56:27.332 24949 25150 D GeckoEditable: getSpanEnd(android.view.inputmethod.ComposingText@832f942) = -1
01-15 16:56:27.332 24949 25150 D GeckoEditable: getSpanStart(android.text.Selection$START@d2d1cd) = 20
01-15 16:56:27.332 24949 25150 D GeckoEditable: getSpanStart(android.text.Selection$END@73dfa82) = 20
01-15 16:56:27.333 24949 25150 D GeckoEditable: length() = 20
01-15 16:56:27.333 24949 25150 D GeckoEditable: getSpanStart(android.text.Selection$START@d2d1cd) = 20
01-15 16:56:27.333 24949 25150 D GeckoEditable: getSpanStart(android.text.Selection$END@73dfa82) = 20
01-15 16:56:27.334 24949 25150 D GeckoEditable: offer: Action(TYPE_REPLACE_TEXT)
01-15 16:56:27.334 24949 25150 D GeckoEditable: icSendComposition("G", 0, 1)
01-15 16:56:27.334 24949 25150 D GeckoEditable: range = 0-1
01-15 16:56:27.334 24949 25150 D GeckoEditable: selection = 1-1
01-15 16:56:27.335 24949 25150 D GeckoEditable: found 1 spans @ 0-1
01-15 16:56:27.335 24949 25150 D GeckoEditable: added 4 : 1 : 0 : 0
01-15 16:56:27.335 24949 24965 I GeckoEditableSupport: IME: IME_REPLACE_TEXT: text="G"
01-15 16:56:27.335 24949 25150 D GeckoEditable: replace(20, 20, "G") = GeckoEditable
01-15 16:56:27.335 24949 25150 D GeckoInputConnection: < setComposingText: true
01-15 16:56:27.336 24949 25150 D GeckoInputConnection: > endBatchEdit()
01-15 16:56:27.336 24949 25150 D GeckoInputConnection: < endBatchEdit: true
01-15 16:56:27.357 24949 24965 I GeckoEditableSupport: IME: NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED
01-15 16:56:27.455 24949 24965 I Gecko : [Parent 24949, Main Thread] WARNING: '!CanHandleEditAction()', file /home/makoto/mozilla-central/editor/libeditor/TextEditRules.cpp, line 287
01-15 16:56:27.455 24949 24965 I Gecko : [Parent 24949, Main Thread] WARNING: Failed to get value: 'NS_SUCCEEDED(rv)', file /home/makoto/mozilla-central/dom/html/nsTextEditorState.cpp, line 2106
01-15 16:56:27.458 24949 24965 I Gecko : [Parent 24949, Main Thread] WARNING: '!aSelection.RangeCount()', file /home/makoto/mozilla-central/editor/libeditor/EditorBase.cpp, line 3556
01-15 16:56:27.459 24949 24965 I Gecko : [Parent 24949, Main Thread] WARNING: '!selectionStartPoint.IsSet()', file /home/makoto/mozilla-central/editor/libeditor/TextEditRules.cpp, line 529
01-15 16:56:27.459 24949 24965 I Gecko : [Parent 24949, Main Thread] WARNING: Failed to selection to after the text node in TextEditor: 'NS_SUCCEEDED(rv)', file /home/makoto/mozilla-central/editor/libeditor/TextEditRules.cpp, line 280
01-15 16:56:27.459 24949 24965 I Gecko : [Parent 24949, Main Thread] WARNING: '!firstRange', file /home/makoto/mozilla-central/editor/libeditor/TextEditRules.cpp, line 809
01-15 16:56:27.459 24949 24965 I Gecko : [Parent 24949, Main Thread] WARNING: 'NS_FAILED(rv)', file /home/makoto/mozilla-central/editor/libeditor/TextEditor.cpp, line 1023
01-15 16:56:27.459 24949 24965 I Gecko : [Parent 24949, Main Thread] WARNING: 'NS_FAILED(rv)', file /home/makoto/mozilla-central/editor/libeditor/TextEditor.cpp, line 1225
01-15 16:56:27.459 24949 24965 I Gecko : [Parent 24949, Main Thread] WARNING: Failed to replace selection with new string: 'NS_SUCCEEDED(rv)', file /home/makoto/mozilla-central/editor/libeditor/TextEditor.cpp, line 1204
01-15 16:56:27.459 24949 24965 I Gecko : [Parent 24949, Main Thread] WARNING: '!aSelection.RangeCount()', file /home/makoto/mozilla-central/editor/libeditor/EditorBase.cpp, line 3556
01-15 16:56:27.459 24949 24965 I Gecko : [Parent 24949, Main Thread] WARNING: '!selectionStartPoint.IsSet()', file /home/makoto/mozilla-central/editor/libeditor/TextEditRules.cpp, line 529
01-15 16:56:27.459 24949 24965 I Gecko : [Parent 24949, Main Thread] WARNING: Failed to selection to after the text node in TextEditor: 'NS_SUCCEEDED(rv)', file /home/makoto/mozilla-central/editor/libeditor/TextEditRules.cpp, line 280
01-15 16:56:27.459 24949 24965 I Gecko : [Parent 24949, Main Thread] WARNING: '!aDocument', file /home/makoto/mozilla-central/editor/libeditor/EditorBase.cpp, line 1947
01-15 16:56:27.459 24949 24965 I Gecko : [Parent 24949, Main Thread] WARNING: NS_ENSURE_TRUE(mShell) failed: file /home/makoto/mozilla-central/layout/generic/nsFrameSelection.cpp, line 1208
01-15 16:56:27.459 24949 24965 I Gecko : [Parent 24949, Main Thread] WARNING: '!firstRange', file /home/makoto/mozilla-central/editor/libeditor/EditorBase.cpp, line 4342
01-15 16:56:27.503 24949 24965 D GeckoEditable: reply: Action(TYPE_REPLACE_TEXT)
01-15 16:56:27.503 24949 25150 D GeckoInputConnection: > onTextChange()
01-15 16:56:27.503 24949 25150 D GeckoInputConnection: < onTextChange

nsTextEditorState seems to use destroyed editor. So InsertTextAsAction will be failure. nsTextEditorState should reinitialize editor or create editor.

Assignee: nobody → m_kato

Why is nsTextEditorState::UnbindFrame called on Android only?

Even if I use PC-site mode, I can reproduce it. I don't know how to do it, but I sometimes see such web apps (i.e., even in PC mode, only Firefox for Android is broken).

(In reply to Masayuki Nakano [:masayuki] (JST, +0900)(sick) from comment #15)

Even if I use PC-site mode, I can reproduce it. I don't know how to do it, but I sometimes see such web apps (i.e., even in PC mode, only Firefox for Android is broken).

input event handler of <textarea> element calls element.scrollTop(). When it causes reflow, editor is destroyed by UnbindFromFrame.

Do you mean that InsertTextAsAction() is called by an input event listener while the editor is temporarily destroyed?

(In reply to Masayuki Nakano [:masayuki] (JST, +0900)(sick) from comment #17)

Do you mean that InsertTextAsAction() is called by an input event listener while the editor is temporarily destroyed?

No, editor is destroyed and initialized again by scrollTop due to reflow. InsertTextAsAction will be called since nsTextEditorState has to set current value via SetText action. So since SetText is failed, editor text is nothing.

Why is SetText failed? Because this reflow occurs during edit action. So selection is cached by bug 1465702, but TextEditor::Init has new selection controller. So selection is invalid for newer root node. So I think we have to update selection cache in Init.

(But I cannot create test case... hmm)

GitLab's comment calls scrollTop on input event handler. The scollTop may cause
reflow. When causing reflow, editor is destroyed and initialized again. Then
nsTextEditorState will set current value to editor. But this is failure.

By bug 1465702, selection is cached in edit action. When initializing editor,
selection controller is updated, so selection into cache becomes invalid. It
means that all selection methods will return error since document is different.

So we should update selection cache when initializing editor.

Also, I cannot create test case for this situation since we have to cause reflow
in input and/or composition event. Do you have any idea?

Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/16ad98f608fe
Update selection cache when initializing editor. r=masayuki
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66

Comment on attachment 9037477 [details]
Bug 1518002 - Update selection cache when initializing editor. r=masayuki

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1465702

User impact if declined: When writing a comment in gitlab.com using Firefox/Android, your cannot input a character in caret position after viewing preview.

we can reproduce this easy on Android, but if content script causes reflow in input or composition event, this occurs even if desktop version.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: 1. Goto website like https://gitlab.com/bytesnz/ls-ignore/issues/new
2. Enter multiple lines of text into textare
3. Click the Preview button above the textarea
4. Click the Write button above the textarea to go back to the editor and textarea
5. Focus somewhere in the textarea and start typing.

  • Result
    Cursor starts jumping after a small pause

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Since Editor uses invalid selection (window.selection) object into editor's cache, we use valid selection instead.

String changes made/needed: no

Attachment #9037477 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9037477 [details]
Bug 1518002 - Update selection cache when initializing editor. r=masayuki

[Triage Comment]
Fixes a painful-sounding regression causing character input to be broken on some sites, especially on Android. Approved for 65.0b13/65.0rc1.

Attachment #9037477 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Off topic though, according to the investigation of Mokoto-san, we are recreating all of nsFrameSelection instance, Selection instances and nsRange instances every reflow which causes reframing an editor. This must be really bad for the reflow performance because of the slowness of allocation and creating nsRange instances. So, perhaps, when editor becomes not necessary to keep those objects, nsTextEditorState should keep them for a while. Then, it should release them when editor won't be recreated. Otherwise, they should be used in the new editor. Note that those objects are hidden from web apps. So, we can reuse them safely.

Verified as fixed on the latest Nightly 66.0a1 (2019-01-21) and on Beta 65.0b13.
Tested with HTC 10 (Android 8.0) and Samsung Galaxy S7 Edge (Android 8.0).

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.