Closed
Bug 1083617
Opened 10 years ago
Closed 10 years ago
Sometimes, keyboard app can't receive the new inputcontext if input focus moved from app to chrome process
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
People
(Reporter: timdream, Assigned: timdream)
References
(Depends on 1 open bug)
Details
Attachments
(1 file, 2 obsolete files)
1.90 KB,
patch
|
timdream
:
review+
bajaj
:
approval-mozilla-b2g34+
|
Details | Diff | Splinter Review |
STR: 1. Go to any app, say Contact -> New Contact, and focus on an input field, e.g. Phone number. 2. Tap on the Rocket bar ("Search the web") to switch the focus to the rocket bar. 3. Type something Expected: 1. The input caret moved to rocket bar, and the layout switch to Search w/o auto suggestions. Actual: 1. Sometimes the caret remained in the Contact app frame, sometimes the caret does moved to search field but the layout is not switched, sometimes everything visually correct but you can't type anything. Note: I suspect this is a regression of bug 1057898, bug 1067264, and bug 1067266, but it's worthy to check if InputMethod API is previously shown this behavior already. qawanted for v2.1 check. I have a patch (see bug 1077124 comment 11) that would bring back the race protection in the keyboard app (the delay removed in bug 1067264). I will file a bug blocks this one to land it. We can decide whether or not we need to revert the behavior of input mgmt later, as the visual effect (have the keyboard start slide away in midway and slide back instantly) can be fixed in CSS with bug 1075306. I would still like to keep bug 1057898 in the tree for switching inputs between the same frame.
Assignee | ||
Updated•10 years ago
|
status-b2g-v2.1:
--- → ?
status-b2g-v2.2:
--- → affected
Comment 1•10 years ago
|
||
Tim, I think i'm seeing your bug here in Flame 2.1 KK. I've not experienced all your different scenarios but i often get the keyboard to not show up when I switch between the phone field and the rocketbar. The carat is visible in the rocketbar but no keyboard pops up so obviously I can't type. I can get this frequently if the phone field is tapped then the rocketbar is tapped shortly after as the numerical keyboard is popping up. I tried this in other apps and found the same behavior in Messages app. Tapping in the contact field then into rocketbar produced this. ***Let me know if you think this is not your bug. Hard to tell since you describe several bahaviors. Repro Rate: 5/10 Environmental Variables: Device: Flame 2.1 KK BuildID: 20141015143144 Gaia: 477a9e61c3edf12f32a62a19d329cd277202cc6b Gecko: 67573e422a0f Version: 34.0 (2.1) Firmware Version: L1TC10011800 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
Assignee | ||
Comment 2•10 years ago
|
||
(In reply to Cody Roesch [:croesch] from comment #1) > ***Let me know if you think this is not your bug. Hard to tell since you > describe several bahaviors. > > Repro Rate: 5/10 Thank you. That is indeed this bug is about. So this is not a regression to the mentioned bug after all but a defect existed in InputMethod API. Let me try to isolate the cause in API after finishing bug 1083638.
blocking-b2g: 2.2? → ---
Component: Gaia::Keyboard → DOM: Device Interfaces
Product: Firefox OS → Core
Assignee | ||
Comment 3•10 years ago
|
||
The same STR can result two different results and this patch address one of them. The Rockbar search field comes is placed in System app (inproc), and when the focus moves from the focused element on remote process to there, the blur message from remote process might sent after the focus message from chrome process. This patch address the bug where the actual result is "keyboard non popping up for search field". Unfortunately I am not sure how to write a mochitest for this -- I have a test script ready that could switch focus between two inputs in two frames, but I can't seem to be able to push the two mozbrowser frames to remote. The second result of this bug is, for some reason, when the rocketbar overlay is shown, instead of focus moving from the app to rocketbar, the focus was moved back to the frame. This can be due to a Gaia bug and I would need to dig deeper.
Attachment #8507651 -
Flags: feedback?(xyuan)
Assignee | ||
Comment 4•10 years ago
|
||
Comment on attachment 8507651 [details] [diff] [review] Patch v1.0 This patch does not work for the situation I described. Need to look deeper. We probably need to land bug 1083638 first coz keyboard app itself is a bit flicky too.
Attachment #8507651 -
Flags: feedback?(xyuan)
Assignee | ||
Comment 5•10 years ago
|
||
I try to find the root cause by annotating forms.js and update omni.ja, however for some reason the forms.js in chrome b2g process is loaded from somewhere else and I can't update it. Will try to build the entire Gecko Monday :'(.
Comment 6•10 years ago
|
||
Hm, there's a single forms.js in gecko Tim. So something else is going on, but repackaging omni.ja should really work.
Assignee | ||
Comment 7•10 years ago
|
||
Update on the alternative approach: try to reproduce this bug with mochitest. So, I modified inputmothod_common.js and added the necessary perfs to push iframes to remote, but the tests timed out. Further annotating forms.js found that it is indeed being loaded but it does not receive any focus event. It might be related to bug 1085217 but neither Kanru and I can sure.
Assignee | ||
Comment 8•10 years ago
|
||
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #5) > Will try to build the entire Gecko Monday :'(. Here is the log from built Gecko, when the bug happens: Tap on the Phone # field in Contacts: I/Gecko ( 9891 contacts): XXXXX 407 blur event I/Gecko ( 9891 contacts): XXXXX 380 focus event I/Gecko ( 9891 contacts): XXXXX 675 fa_handleFocus I/Gecko ( 9891 contacts): XXXXX 681 fa_handleFocus I/Gecko ( 9891 contacts): XXXXX 685 fa_handleFocusSync I/Gecko ( 9891 contacts): XXXXX 690 focus true I/Gecko ( 9569 keyboard): XXXXX MozKeyboard.js inputcontext {"contextId":1,"type":"tel","choices":null,"value":"","inputmode":"","selectionStart":0,"selectionEnd":0,"max":"","min":"","lang":"","textBeforeCursor":"","textAfterCursor":""} Tap on Rocket bar: I/Gecko ( 9235 system ): XXXXX 407 blur event I/Gecko ( 9235 system ): XXXXX 380 focus event I/Gecko ( 9235 system ): XXXXX 675 fa_handleFocus I/Gecko ( 9235 system ): XXXXX 681 fa_handleFocus I/Gecko ( 9891 contacts): XXXXX 407 blur event I/Gecko ( 9891 contacts): XXXXX 408 blur event I/Gecko ( 9891 contacts): XXXXX 696 fa_unhandleFocus I/Gecko ( 9891 contacts): XXXXX 407 blur event I/Gecko ( 9891 contacts): XXXXX 407 blur event I/Gecko ( 9235 system ): XXXXX 685 fa_handleFocusSync I/Gecko ( 9235 system ): XXXXX 690 focus true I/Gecko ( 9569 keyboard): XXXXX MozKeyboard.js inputcontext {"contextId":3,"type":"search","choices":null,"value":"","inputmode":"verbatim","selectionStart":0,"selectionEnd":0,"max":"","min":"","lang":"","textBeforeCursor":"","textAfterCursor":""} I/Gecko ( 9891 contacts): XXXXX 703 fa_unhandleFocusSync I/Gecko ( 9891 contacts): XXXXX 710 blur I/Gecko ( 9569 keyboard): XXXXX MozKeyboard.js inputcontext null This indeed confirms comment 3 but I don't know why my patch doesn't work. Will look deeper tomorrow.
Assignee | ||
Comment 9•10 years ago
|
||
Here is a more detailed log and annotation, which confirms comment 3 is indeed correct but we have the same race in System app input management as well: When the focus changes, focus message reaches the keyboard sooner than the blur message from oop frame: I/Gecko ( 2058): XXXXX 407 blur event I/Gecko ( 2058): XXXXX 380 focus event I/Gecko ( 2622): XXXXX 407 blur event I/Gecko ( 2622): XXXXX 408 blur event I/Gecko ( 2622): XXXXX 696 fa_unhandleFocus I/Gecko ( 2058): XXXXX 675 fa_handleFocus I/Gecko ( 2058): XXXXX 681 fa_handleFocus I/Gecko ( 2622): XXXXX 407 blur event I/Gecko ( 2622): XXXXX 407 blur event I/Gecko ( 2058): XXXXX 685 fa_handleFocusSync I/Gecko ( 2622): XXXXX 703 fa_unhandleFocusSync I/Gecko ( 2058): XXXXX 690 focus true I/Gecko ( 2622): XXXXX 710 blur I/Gecko ( 2058): XXXXX 252 forward event Keyboard:FocusChange{"target":{},"name":"Forms:Input","sync":false,"json":{"contextId":7,"type":"search","choices":null,"value":"","inputmode":"verbatim","selectionStart":0,"selectionEnd":0,"max":"","min":"","lang":"","textBeforeCursor":"","textAfterCursor":""},"data":{"contextId":7,"type":"search","choices":null,"value":"","inputmode":"verbatim","selectionStart":0,"selectionEnd":0,"max":"","min":"","lang":"","textBeforeCursor":"","textAfterCursor":""},"objects":{}} I/Gecko ( 2058): XXXXX 267 send I/Gecko ( 2387): XXXXX MozKeyboard 225 I/Gecko ( 2387): XXXXX MozKeyboard.js inputcontext {"contextId":7,"type":"search","choices":null,"value":"","inputmode":"verbatim","selectionStart":0,"selectionEnd":0,"max":"","min":"","lang":"","textBeforeCursor":"","textAfterCursor":""} I/Gecko ( 2058): XXXXX 869 browser elememt true I/Gecko ( 2387): XXXXX 1123 browser element child true I/Gecko ( 2058): XXXXX 252 forward event Keyboard:FocusChange{"target":{},"name":"Forms:Input","sync":false,"json":{"type":"blur"},"data":{"type":"blur"},"objects":{}} I/Gecko ( 2058): XXXXX 260 skip ... and the patch in comment 3 will work and block the blur message I/Gecko ( 2058): XXXXX 252 forward event Keyboard:SelectionChange{"target":{},"name":"Forms:SelectionChange","sync":false,"json":{"selectionStart":0,"selectionEnd":0,"textBeforeCursor":"","textAfterCursor":"","changed":true},"data":{"selectionStart":0,"selectionEnd":0,"textBeforeCursor":"","textAfterCursor":"","changed":true},"objects":{}} I/Gecko ( 2058): XXXXX 267 send I/Gecko ( 2058): XXXXX 252 forward event Keyboard:GetText:Result:OK{"target":{},"name":"Forms:GetText:Result:OK","sync":false,"json":{"requestId":"id{7d4657eb-e83c-4435-88c8-b7b1dd8f4a7d}","text":""},"data":{"requestId":"id{7d4657eb-e83c-4435-88c8-b7b1dd8f4a7d}","text":""},"objects":{}} I/Gecko ( 2058): XXXXX 267 send I/Gecko ( 2058): XXXXX 252 forward event Keyboard:GetText:Result:OK{"target":{},"name":"Forms:GetText:Result:OK","sync":false,"json":{"requestId":"id{3f1e9262-1230-4c23-b558-56fe59b25d1b}","text":""},"data":{"requestId":"id{3f1e9262-1230-4c23-b558-56fe59b25d1b}","text":""},"objects":{}} I/Gecko ( 2058): XXXXX 267 send ... however, we'll receive an setInputMethodActive(false) from input mgmt. I/Gecko ( 2058): XXXXX 869 browser elememt false I/Gecko ( 2387): XXXXX 1123 browser element child false I/Gecko ( 2387): XXXXX MozKeyboard 327 I/Gecko ( 2387): XXXXX MozKeyboard.js inputcontext null I will clone a bug and fix this on the input management side.
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #9) > I will clone a bug and fix this on the input management side. bug 1090034.
Summary: Focus between two frames will fail in both keyboard and InputMethod API, affecting Rocketbar → Sometimes, keyboard app can't receive the new inputcontext if input focus moved from app to chrome process
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8507651 [details] [diff] [review] Patch v1.0 I decide to set this for review. -- I can't write any test with mochitest because of the problem documented in bug 1090032 -- We can't manually verify the fix unless input mgmt is also fixed, in bug 1090034 Let's attack the problem one at the time. https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=4e2193516ce8
Attachment #8507651 -
Flags: review?(xyuan)
Assignee | ||
Comment 12•10 years ago
|
||
After re-read the code again I realize my judgement in comment 9 is not correct. My patch did not stop chrome event from being sent out to input mgmt which caused input mgmt to call setInputMethodActive(false). This is the correct the patch. However the reason for me not to write a mochitest main valid.
Attachment #8507651 -
Attachment is obsolete: true
Attachment #8507651 -
Flags: review?(xyuan)
Attachment #8512454 -
Flags: review?(xyuan)
Assignee | ||
Comment 13•10 years ago
|
||
Try run of patch v1.1 https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=917828d454ac
Assignee | ||
Comment 15•10 years ago
|
||
[Blocking Requested - why for this release]: bug 1087556 was nominated.
blocking-b2g: --- → 2.1?
Updated•10 years ago
|
blocking-b2g: 2.1? → 2.1+
Comment 16•10 years ago
|
||
Comment on attachment 8512454 [details] [diff] [review] Patch v1.1 Review of attachment 8512454 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/inputmethod/Keyboard.jsm @@ +248,5 @@ > } > }, > > forwardEvent: function keyboardForwardEvent(newEventName, msg) { > + var mm = msg.target.QueryInterface(Ci.nsIFrameLoaderOwner) nit: use |let| instead of |var| @@ +273,5 @@ > > handleFocusChange: function keyboardHandleFocusChange(msg) { > + var sent = this.forwardEvent('Keyboard:FocusChange', msg); > + > + if (!sent) { nit: The |sent| variable is not necessary and you can combine the above two lines into one.
Attachment #8512454 -
Flags: review?(xyuan) → review+
Assignee | ||
Comment 17•10 years ago
|
||
(In reply to Yuan Xulei [:yxl] from comment #16) > @@ +273,5 @@ > > > > handleFocusChange: function keyboardHandleFocusChange(msg) { > > + var sent = this.forwardEvent('Keyboard:FocusChange', msg); > > + > > + if (!sent) { > > nit: The |sent| variable is not necessary and you can combine the above two > lines into one. I think this will reduce the readability of the code, I will keep this and simply do s/var/let/. Is that good to you?
Flags: needinfo?(xyuan)
Assignee | ||
Comment 18•10 years ago
|
||
Attachment #8512454 -
Attachment is obsolete: true
Attachment #8513269 -
Flags: review+
Comment 19•10 years ago
|
||
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #17) > (In reply to Yuan Xulei [:yxl] from comment #16) > > @@ +273,5 @@ > > > > > > handleFocusChange: function keyboardHandleFocusChange(msg) { > > > + var sent = this.forwardEvent('Keyboard:FocusChange', msg); > > > + > > > + if (!sent) { > > > > nit: The |sent| variable is not necessary and you can combine the above two > > lines into one. > > I think this will reduce the readability of the code, I will keep this and > simply do s/var/let/. Is that good to you? It's fine for me. Personally speaking, I don't see any readability issue without |sent| variable :)
Flags: needinfo?(xyuan)
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 20•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/52cdf66f6666
Keywords: checkin-needed
Comment 21•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/52cdf66f6666
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Assignee | ||
Comment 22•10 years ago
|
||
Will nominate for uplift after seeing the patch on daily OTA.
Flags: needinfo?(timdream)
Assignee | ||
Updated•10 years ago
|
status-b2g-v2.1:
affected → ---
status-b2g-v2.2:
affected → ---
status-firefox34:
--- → affected
status-firefox36:
--- → fixed
Flags: needinfo?(timdream)
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(timdream)
Assignee | ||
Comment 23•10 years ago
|
||
Comment on attachment 8513269 [details] [diff] [review] Patch for commit [Approval Request Comment] Bug caused by (feature/regressing bug #): This is an old issue since the creating of the API, but commonly trigger-able since the introduction of the rocket bar. User impact if declined: The STR will fail, creating confusing and annoy user experience. Testing completed: tested manually on m-c and locally Risk to taking this patch (and alternatives if risky): There is no alternative. This is the minimal patch possible to workaround the bug. String or UUID changes made by this patch: None.
Flags: needinfo?(timdream)
Attachment #8513269 -
Flags: approval-mozilla-b2g34?
Updated•10 years ago
|
Updated•10 years ago
|
Attachment #8513269 -
Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
Comment 25•10 years ago
|
||
Tim, I was trying to verify the fix for this bug, but found another issue. After STR #2 on Comment 0, the keyboard did not pop up until the user taps the rocket bar. I'm not sure if this is a separate or related issue. This only happens on Flame 2.1, and once the user taps the rocket bar, the keyboard is fully functional. Here's the video: http://youtu.be/ptDfAp--v-Y Please let me know if this issue prevents verifying the original bug. Thanks! Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash) BuildID: 20141103001220 Gaia: 027a7de0c95320cea0579bfd1a4ceef3e9038f34 Gecko: ffecb2be228b Gonk: 48835395daa6a49b281db62c50805bd6ca24077e Version: 34.0 (2.1) Firmware Version: v188 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
Flags: needinfo?(timdream)
Updated•10 years ago
|
QA Contact: croesch
Assignee | ||
Comment 26•10 years ago
|
||
Yeojin, That is not related to this bug I think. I don't think it would always happen either -- you should be able to verify this bug.
Flags: needinfo?(timdream)
Comment 27•10 years ago
|
||
This issue is verified fixed on Flame 2.1. Result: The keyboard is displayed and fully functional when switched from an app to rocket bar. Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash) BuildID: 20141103001220 Gaia: 027a7de0c95320cea0579bfd1a4ceef3e9038f34 Gecko: ffecb2be228b Gonk: 48835395daa6a49b281db62c50805bd6ca24077e Version: 34.0 (2.1) Firmware Version: v188 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 ========================================= This issue still reproduces on Flame 2.2. STR: 1. Open Contacts > Add contact. 2. Tap on the "Phone" field, and type a few numbers. 3. Tap on the rocket bar. Result: The keyboard does not show up, even after the user taps the search field. Repro rate: 3/5 Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash) BuildID: 20141104040207 Gaia: 3c50520982560ccba301474d1ac43706138fc851 Gecko: 54d05732f29b Gonk: 48835395daa6a49b281db62c50805bd6ca24077e Version: 36.0a1 (2.2) Firmware Version: v188 User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?][failed-verification]
Flags: needinfo?(ktucker)
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?][failed-verification] → [QAnalyst-Triage+][failed-verification]
Flags: needinfo?(ktucker)
Keywords: verifyme
Comment 28•10 years ago
|
||
(In reply to Yeojin Chung [:YeojinC] from comment #27) > This issue still reproduces on Flame 2.2. > > STR: > 1. Open Contacts > Add contact. > 2. Tap on the "Phone" field, and type a few numbers. > 3. Tap on the rocket bar. > > Result: The keyboard does not show up, even after the user taps the search > field. > Repro rate: 3/5 > > Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash) > BuildID: 20141104040207 > Gaia: 3c50520982560ccba301474d1ac43706138fc851 > Gecko: 54d05732f29b > Gonk: 48835395daa6a49b281db62c50805bd6ca24077e > Version: 36.0a1 (2.2) > Firmware Version: v188 > User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0 Filed a new bug regarding the issue on Flame 2.2. Bug 1097936
QA Whiteboard: [QAnalyst-Triage+][failed-verification] → [QAnalyst-Triage?][failed-verification]
Flags: needinfo?(ktucker)
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?][failed-verification] → [QAnalyst-Triage+][failed-verification]
Flags: needinfo?(ktucker)
You need to log in
before you can comment on or make changes to this bug.
Description
•