Closed Bug 1596920 Opened 5 years ago Closed 5 years ago

Text entry in Gecko is broken on the Oculus Go in Firefox Reality

Tracking

(firefox75 fixed, firefox77 fixed)

Status:

RESOLVED FIXED

Milestone:

mozilla75

Tracking Flags:

Tracking

Status

firefox75

---

fixed

firefox77

---

fixed

People

(Reporter: rbarker, Assigned: m_kato)

References

Details

(Whiteboard: [geckoview:m1912][geckoview:m74][geckoview:m75][geckoview:m76][geckoview:m77][geckoview:m78])

Attachments

(5 files)

Actual results. 5 years ago Randall Barker [:rbarker] 199.32 KB, image/jpeg		Details
Bug 1596920 - Waiting for text change when keypress is dispatched. r? 5 years ago Makoto Kato [:m_kato] 47 bytes, text/x-phabricator-request		Details \| Review
Logcat with flags 5 years ago Randall Barker [:rbarker] 867.74 KB, text/plain		Details
type.mp4 5 years ago Randall Barker [:rbarker] 3.27 MB, video/mp4		Details
Bug 1596920 - Don't set selection on dispatching key event if unnecessary. 5 years ago Makoto Kato [:m_kato] 47 bytes, text/x-phabricator-request		Details \| Review

Randall Barker [:rbarker]

Reporter

Description

•

5 years ago

Attached image Actual results. — Details

Steps to Reproduce:

Open settings and click account button
attempt to enter email address mr.noob@aol.com

Actual result:
Text displayed is mroobaol.com@.n
Expected result:
Text should be displayed as entered.

Notes:
FxR is using GeckoView Nightly 72.0.20191104094118

I have not seen this issue on faster devices such as the Oculus Quest or HTC Focus.

Randall Barker [:rbarker]

Reporter

Comment 1

•

5 years ago

FxR issue: https://github.com/MozillaReality/FirefoxReality/issues/2290

Emily Toop (:fluffyemily)

Updated

•

5 years ago

Rank: 5

Depends on: 1569007

Priority: -- → P2

Emily Toop (:fluffyemily)

Updated

•

5 years ago

Whiteboard: [geckoview:m1912]

Makoto Kato [:m_kato]

Assignee

Comment 2

•

5 years ago

Gecko Thread

11-18 11:20:19.946  4701  4722 I GeckoEditableSupport: IME: IME_REPLACE_TEXT: text="."
11-18 11:20:20.000  4701  4722 I GeckoEditableSupport: IME: NotifyIMEOfTextChange: s=9, oe=9, ne=10
11-18 11:20:20.001  4701  4722 I GeckoEditableSupport: IME: NOTIFY_IME_OF_SELECTION_CHANGE
11-18 11:20:20.024  4701  4722 I GeckoEditableSupport: IME: IME_REPLACE_TEXT: text="c"
11-18 11:20:20.080  4701  4722 I GeckoEditableSupport: IME: IME_REPLACE_TEXT: text="o"
11-18 11:20:20.105  4701  4722 I GeckoEditableSupport: IME: NotifyIMEOfTextChange: s=10, oe=10, ne=12 <--- merged "c" and "o" event
11-18 11:20:20.107  4701  4722 I GeckoEditableSupport: IME: NOTIFY_IME_OF_SELECTION_CHANGE
11-18 11:20:20.144  4701  4722 I GeckoEditableSupport: IME: IME_REPLACE_TEXT: text="m"
11-18 11:20:20.181  4701  4722 I GeckoEditableSupport: IME: NotifyIMEOfTextChange: s=10, oe=10, ne=11

IC and Java thread

11-18 11:20:19.929  4539  4554 D GeckoEditable: reply: Action(TYPE_SET_SPAN)
11-18 11:20:19.951  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@9b3253) = 0
11-18 11:20:19.952  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@a9be790) = 0
11-18 11:20:19.952  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@e4fa889) = 0
11-18 11:20:19.952  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@7b26178) = 0
11-18 11:20:19.953  4539  4539 D GeckoEditable: sendKeyEvent(KeyEvent { action=ACTION_UP, keyCode=KEYCODE_PERIOD, scanCode=0, metaState=0, flags=0x0, repeatCount=0, eventTime=613010, downTime=613010, deviceId=-1, source=0x101 }, 1, 0)
11-18 11:20:19.955  4539  4539 D GeckoEditable: offer: Action(TYPE_EVENT)
Offering replace text
11-18 11:20:19.984  4539  4555 D GeckoEditable: reply: Action(TYPE_REPLACE_TEXT)
11-18 11:20:19.985  4539  4554 D GeckoEditable: reply: Action(TYPE_SET_SPAN)
11-18 11:20:19.985  4539  4554 D GeckoEditable: discarding stale set span call
11-18 11:20:19.993  4539  4539 D GeckoEditable: getSpanStart(android.text.Selection$START@f5d0781) = 10
11-18 11:20:19.993  4539  4539 D GeckoEditable: getSpanStart(android.text.Selection$END@1abb026) = 10
11-18 11:20:19.993  4539  4539 D GeckoEditable: getSpanStart(android.text.NoCopySpan$Concrete@f4f31b7) = -1
11-18 11:20:19.993  4539  4539 D GeckoEditable: getSpanEnd(android.text.NoCopySpan$Concrete@f4f31b7) = -1
11-18 11:20:19.994  4539  4539 D GeckoEditable: offer: Action(TYPE_SET_SPAN)
11-18 11:20:19.997  4539  4539 D GeckoEditable: setSpan(android.text.NoCopySpan$Concrete@30d908d, 10, 10, 17)
11-18 11:20:19.997  4539  4539 D GeckoEditable: offer: Action(TYPE_REPLACE_TEXT)
11-18 11:20:19.998  4539  4539 D GeckoEditable: icSendComposition(): no composition
11-18 11:20:19.999  4539  4774 D GeckoEditable: onDefaultKeyEvent(action=1, keyCode=56, metaState=0, time=613010, repeatCount=0)
11-18 11:20:20.000  4539  4539 D GeckoEditable: sending: KeyEvent { action=ACTION_DOWN, keyCode=KEYCODE_C, scanCode=0, metaState=0, flags=0x0, repeatCount=0, eventTime=614068, downTime=614068, deviceId=-1, source=0x101 }
11-18 11:20:20.003  4539  4539 D GeckoEditable: replace(10, 10, "c") = GeckoEditable
11-18 11:20:20.004  4539  4539 D GeckoEditable: getSpanStart(android.text.NoCopySpan$Concrete@30d908d) = 10
XXXX 11-18 11:20:20.004  4539  4539 D GeckoEditable: getSpanStart(android.text.Selection$END@1abb026) = 11 <--- When typeing "c" selection on Java is 11 by repalce affer
11-18 11:20:20.004  4539  4539 D GeckoEditable: offer: Action(TYPE_SET_SPAN)
11-18 11:20:20.007  4539  4539 D GeckoEditable: setSpan(android.text.NoCopySpan$Concrete@8dfb242, 10, 11, 33)
11-18 11:20:20.007  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@9b3253) = 0
11-18 11:20:20.008  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@a9be790) = 0
11-18 11:20:20.008  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@e4fa889) = 0
11-18 11:20:20.011  4539  4832 D GeckoEditable: onTextChange(".", 9, 9)
XXX 11-18 11:20:20.016  4539  4555 D GeckoEditable: onSelectionChange(10, 10) <--- "." is set in Gecko now, then selection on Gecko is 10. Most senario isn't sycned, but icSyncShadowText syncs text due to no data in our queue.
11-18 11:20:20.018  4539  4640 D GeckoEditable: reply: Action(TYPE_EVENT)
11-18 11:20:20.023  4539  4554 D GeckoEditable: reply: Action(TYPE_SET_SPAN)
11-18 11:20:20.036  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@9b3253) = 0
11-18 11:20:20.036  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@a9be790) = 0
11-18 11:20:20.037  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@e4fa889) = 0
11-18 11:20:20.037  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@7b26178) = 0
11-18 11:20:20.037  4539  4539 D GeckoEditable: sendKeyEvent(KeyEvent { action=ACTION_UP, keyCode=KEYCODE_C, scanCode=0, metaState=0, flags=0x0, repeatCount=0, eventTime=613010, downTime=613010, deviceId=-1, source=0x101 }, 1, 0)
11-18 11:20:20.041  4539  4539 D GeckoEditable: offer: Action(TYPE_EVENT)
11-18 11:20:20.046  4539  4774 D GeckoEditable: reply: Action(TYPE_REPLACE_TEXT)
11-18 11:20:20.047  4539  4832 D GeckoEditable: reply: Action(TYPE_SET_SPAN)
11-18 11:20:20.047  4539  4832 D GeckoEditable: discarding stale set span call
11-18 11:20:20.056  4539  4640 D GeckoEditable: onDefaultKeyEvent(action=1, keyCode=31, metaState=0, time=613010, repeatCount=0)
11-18 11:20:20.058  4539  4554 D GeckoEditable: reply: Action(TYPE_EVENT)
XXX 11-18 11:20:20.060  4539  4539 D GeckoEditable: getSpanStart(android.text.Selection$START@f5d0781) = 10 <--- selection on Java is 10 unfortunately
11-18 11:20:20.060  4539  4539 D GeckoEditable: getSpanStart(android.text.Selection$END@1abb026) = 10
...
11-18 11:20:20.073  4539  4539 D GeckoEditable: offer: Action(TYPE_REPLACE_TEXT)
11-18 11:20:20.073  4539  4539 D GeckoEditable: icSendComposition(): no composition
11-18 11:20:20.073  4539  4539 D GeckoEditable: sending: KeyEvent { action=ACTION_DOWN, keyCode=KEYCODE_O, scanCode=0, metaState=0, flags=0x0, repeatCount=0, eventTime=614141, downTime=614141, deviceId=-1, source=0x101 }
11-18 11:20:20.080  4539  4539 D GeckoEditable: replace(10, 10, "o") = GeckoEditable
11-18 11:20:20.080  4539  4539 D GeckoEditable: getSpanStart(android.text.NoCopySpan$Concrete@30d908d) = 10
11-18 11:20:20.080  4539  4539 D GeckoEditable: getSpanStart(android.text.Selection$END@1abb026) = 11
11-18 11:20:20.081  4539  4539 D GeckoEditable: offer: Action(TYPE_SET_SPAN)
11-18 11:20:20.083  4539  4539 D GeckoEditable: setSpan(android.text.NoCopySpan$Concrete@8dfb242, 10, 11, 33)
11-18 11:20:20.083  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@9b3253) = 0
11-18 11:20:20.084  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@a9be790) = 0
11-18 11:20:20.084  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@e4fa889) = 0
11-18 11:20:20.101  4539  4555 D GeckoEditable: reply: Action(TYPE_REPLACE_TEXT)
11-18 11:20:20.102  4539  4640 D GeckoEditable: reply: Action(TYPE_SET_SPAN)
11-18 11:20:20.104  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@9b3253) = 0
11-18 11:20:20.104  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@a9be790) = 0
11-18 11:20:20.104  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@e4fa889) = 0
11-18 11:20:20.104  4539  4539 D GeckoEditable: getSpanFlags(android.text.NoCopySpan$Concrete@7b26178) = 0
11-18 11:20:20.105  4539  4539 D GeckoEditable: sendKeyEvent(KeyEvent { action=ACTION_UP, keyCode=KEYCODE_O, scanCode=0, metaState=0, flags=0x0, repeatCount=0, eventTime=613010, downTime=613010, deviceId=-1, source=0x101 }, 1, 0)
11-18 11:20:20.106  4539  4539 D GeckoEditable: offer: Action(TYPE_EVENT)
11-18 11:20:20.119  4539  4554 D GeckoEditable: onTextChange("oc", 10, 10)

James Willcox (:snorp) (jwillcox@mozilla.com) (he/him)

Updated

•

5 years ago

Priority: P2 → P1

Emily Toop (:fluffyemily)

Updated

•

5 years ago

Component: General → Graphics: WebRender

Product: GeckoView → Core

Emily Toop (:fluffyemily)

Updated

•

5 years ago

Component: Graphics: WebRender → General

Product: Core → GeckoView

Makoto Kato [:m_kato]

Assignee

Updated

•

5 years ago

Assignee: nobody → m_kato

Emily Toop (:fluffyemily)

Updated

•

5 years ago

Whiteboard: [geckoview:m1912] → [geckoview:m1912] [geckoview:m74]

Makoto Kato [:m_kato]

Assignee

Comment 3

•

5 years ago

WIP
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0a6207237e32bd496c9cce023ff8cb8dd33091e4

Emily Toop (:fluffyemily)

Updated

•

5 years ago

Whiteboard: [geckoview:m1912] [geckoview:m74] → [geckoview:m1912][geckoview:m74][geckoview:m75]

Makoto Kato [:m_kato]

Assignee

Comment 4

•

5 years ago

Attached file Bug 1596920 - Waiting for text change when keypress is dispatched. r? — Details

Pulsebot

Comment 5

•

5 years ago

Pushed by m_kato@ga2.so-net.ne.jp: https://hg.mozilla.org/integration/autoland/rev/3b50706b1bad Waiting for text change when keypress is dispatched. r=geckoview-reviewers,esawin

Cristina Coroiu [:ccoroiu]

Comment 6

•

5 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/3b50706b1bad

Status: NEW → RESOLVED

Closed: 5 years ago

status-firefox75: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla75

Randall Barker [:rbarker]

Reporter

Comment 7

•

5 years ago

I can still easily reproduce this issue in FxR using GeckoView Nightly 75.0.20200221095110.

Status: RESOLVED → REOPENED

Resolution: FIXED → ---

Makoto Kato [:m_kato]

Assignee

Comment 8

•

5 years ago

(In reply to Randall Barker [:rbarker] from comment #7)

I can still easily reproduce this issue in FxR using GeckoView Nightly 75.0.20200221095110.

Could you attach adb log with the latest GV? (set DEBUG_ANDROID_IME=1 on GeckoEditoable.cpp and set DEBUG value to true for GeckoEditable.java and GeckoInputConnection.java). When I emulate old device that uses Android arm emulator, this cannot reproduce this after landing my fix.

Flags: needinfo?(rbarker)

Emily Toop (:fluffyemily)

Updated

•

5 years ago

Whiteboard: [geckoview:m1912][geckoview:m74][geckoview:m75] → [geckoview:m1912][geckoview:m74][geckoview:m75][geckoview:m76]

Randall Barker [:rbarker]

Reporter

Comment 9

•

5 years ago

I can no longer reproduce this issue. Maybe I was on an older version of FxR when I reproduced.

Status: REOPENED → RESOLVED

Closed: 5 years ago → 5 years ago

Flags: needinfo?(rbarker)

Resolution: --- → FIXED

Randall Barker [:rbarker]

Reporter

Comment 10

•

5 years ago

Figures, as soon as I close this, I am able to reproduce. I'll try and collect the logs you asked for now. Sorry.

Status: RESOLVED → REOPENED

Resolution: FIXED → ---

Randall Barker [:rbarker]

Reporter

Comment 11

•

5 years ago

Attached file Logcat with flags — Details

:m_kato, I've attached the logcat from a build of GeckoView from the latest in m-c. Hope this helps.

Flags: needinfo?(m_kato)

Randall Barker [:rbarker]

Reporter

Comment 12

•

5 years ago

Attached video type.mp4 — Details

Video recording of FxR during logcat capture.

Makoto Kato [:m_kato]

Assignee

Comment 13

•

5 years ago

Thanks, I am looking.

Flags: needinfo?(m_kato)

Emily Toop (:fluffyemily)

Updated

•

5 years ago

Whiteboard: [geckoview:m1912][geckoview:m74][geckoview:m75][geckoview:m76] → [geckoview:m1912][geckoview:m74][geckoview:m75][geckoview:m76][geckoview:m77]

Makoto Kato [:m_kato]

Assignee

Comment 14

•

5 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3fe89fb0c3f710caec5068949af98a551f254826

Makoto Kato [:m_kato]

Assignee

Comment 15

•

5 years ago

Attached file Bug 1596920 - Don't set selection on dispatching key event if unnecessary. — Details

Actually we emulate key event (down, press and up) from replace transaction of
Editable. When dispatching key press, we always update current caret position.

Most situations is the following.

Dispatch keypress
Dispatch another keypress
Receive merged text/selection change result by 1. and 2.
Sync shadow (Java's Editable) text with 3.'s result. It means selection is
correct position now.
Dispatch keypress with correct position.

When this issue occurs, the following situation occurs.

Dispatch keypress
Dispatch another keypress
Receive text/selection change result of 1.
Sync shadow (Java's Editable) text with 3.'s result. It means selection is
old position now.
Dispatch another keypress with old position.
Receive text/selection change result of 2.
Receive text/selection change result of 5.

So when dispatching key press, we shouldn't always update selection if unnecessary.
Because selection range is already often set before dispatching key press.

Pulsebot

Comment 17

•

5 years ago

Pushed by m_kato@ga2.so-net.ne.jp: https://hg.mozilla.org/integration/autoland/rev/27da136bf5a9 Don't set selection on dispatching key event if unnecessary. r=geckoview-reviewers,snorp

Emily Toop (:fluffyemily)

Updated

•

5 years ago

Whiteboard: [geckoview:m1912][geckoview:m74][geckoview:m75][geckoview:m76][geckoview:m77] → [geckoview:m1912][geckoview:m74][geckoview:m75][geckoview:m76][geckoview:m77][geckoview:m78]

Cristian Brindusan [:cbrindusan]

Comment 18

•

5 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/27da136bf5a9

Status: REOPENED → RESOLVED

Closed: 5 years ago → 5 years ago

status-firefox77: --- → fixed

Resolution: --- → FIXED

You need to log in before you can comment on or make changes to this bug.