Closed
Bug 812115
Opened 12 years ago
Closed 12 years ago
[Keyboard] Keyboard quit later than the app
Categories
(Firefox OS Graveyard :: Gaia::Keyboard, defect, P4)
Firefox OS Graveyard
Gaia::Keyboard
Tracking
(blocking-basecamp:-)
VERIFIED
FIXED
blocking-basecamp | - |
People
(Reporter: johnshih.bugs, Assigned: ttaubert)
Details
(Keywords: polish)
Attachments
(1 file, 1 obsolete file)
355 bytes,
text/html
|
vingtetun
:
review+
ttaubert
:
feedback+
vingtetun
:
approval-gaia-v1+
|
Details |
## Environment : Unagi phone, build 2012-11-15 Build info: * "gaia master" revision= ab2a7c7ae90a0a4764f51b5eabd1c2d5e6117c9c * "mozilla-aurora" revision= 0d76df6f808d Video:http://www.youtube.com/watch?v=tIo3GUNBcHY&feature=youtu.be ## Repro : 1. Launch the contact app 2. Press "+" to add a contact 3. Focus on arbitrary field to launch the keyboard 4. press the home key to quit the app ## Expected: * Back to homescreen ## Actual: * Back to homescreen with keyboard still shows up for a short time
Assignee | ||
Updated•12 years ago
|
OS: Windows 7 → All
Hardware: x86_64 → All
Comment 1•12 years ago
|
||
I saw this too a few times - good work getting STR here.
Component: General → Gaia
Comment 2•12 years ago
|
||
Keyboard will be hidden after keyboard app receives onfocuschange event with |type == 'blur'|. However, for OOP apps, we will introduce 40ms delay for hiding the keyboard. http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/forms.js#109 https://github.com/mozilla-b2g/gaia/blob/master/apps/keyboard/js/keyboard.js#L404 Maybe we can hide keyboard-frame first while we invoke |closeWindow| in window manager.
Updated•12 years ago
|
blocking-basecamp: ? → +
Priority: -- → P2
Assignee | ||
Comment 3•12 years ago
|
||
We could let forms.js listen for 'appwillclose' and blur the currently focused element. That would instantly hide the keyboard. The only problem is that's a synthetic event and doesn't reach the content script (forms.js). Not sure how to work around that, yet.
Comment 4•12 years ago
|
||
(In reply to Tim Taubert [:ttaubert] from comment #3) > We could let forms.js listen for 'appwillclose' and blur the currently > focused element. That would instantly hide the keyboard. The only problem is > that's a synthetic event and doesn't reach the content script (forms.js). > Not sure how to work around that, yet. Let's move some logic on the system app side if needed.
Assignee: nobody → ttaubert
Updated•12 years ago
|
Component: Gaia → Gaia::System::Keyboard
Assignee | ||
Comment 5•12 years ago
|
||
So the problem here is I think the communication overhead between processes. I can't reproduce the problem all the time but maybe in 30% of all tries. When pressing the home button, WindowManager.closeWindow() is called. Sometimes it takes up to 200+ms for the blur event to be handled in forms.js. 100-150ms later the async message arrives in Keyboard.jsm and Keyboard:FocusChange gets broadcasted. About 130ms after that the onfocuschange event is dispatched and the keyboard app starts a timeout with 20ms which is then fired 150-200ms(!) later. That's why it takes up 600ms until the keyboard is hidden. We also fade it out which adds a couple of perceived time as well.
Comment 6•12 years ago
|
||
Marking for C2, given this meets the criteria of known P1/P2 blocking-basecamp+ bugs at the end of C1.
Target Milestone: --- → B2G C2 (20nov-10dec)
Assignee | ||
Comment 7•12 years ago
|
||
I suspect that GC is one reason for the huge lag. When putting an app into the background I see this: GC(T+65.9) Total Time: 380.0ms, Compartments Collected: 33, Total Compartments: 33, MMU (20ms): 0%, MMU (50ms): 0%, SCC Sweep Total: 232.2ms, SCC Sweep Max Pause: 215.9ms, Max Pause: 309.3ms, Allocated: 5MB, +Chunks: 1, -Chunks: 0 This is followed by a couple of ~30ms GCs with "Reason: MEM_PRESSURE". When just opening the contacts app I see three GCs: 360ms, 330ms and 240ms. All because of MEM_PRESSURE.
Target Milestone: B2G C2 (20nov-10dec) → ---
Assignee | ||
Updated•12 years ago
|
Target Milestone: --- → B2G C2 (20nov-10dec)
Assignee | ||
Comment 8•12 years ago
|
||
Well, even without calling frame.setVisible(false) - which I think would activate GC, right? - the keyboard is still visible way too long after the app has disappeared. I don't see any CC/GC logs so maybe that's not it.
Comment 9•12 years ago
|
||
Thanks for looking into this. One thing I can think off that contribute the delay is that we always do the keyboard hidden transition (regressed recently so it doesn't really look like a transition) regardless the reason of hiding the keyboard (because of real blur() or app close). That can be fixed in keyboard_manager.js along. (In reply to Tim Taubert [:ttaubert] from comment #3) > We could let forms.js listen for 'appwillclose' and blur the currently > focused element. That would instantly hide the keyboard. The only problem is > that's a synthetic event and doesn't reach the content script (forms.js). > Not sure how to work around that, yet. Please don't make chrome script listens to events generated in System app. We need to keep the two decoupled (although we already did that for a few cases).
Comment 10•12 years ago
|
||
blocking-, would take a patch for it but not blocking
Updated•12 years ago
|
Target Milestone: B2G C2 (20nov-10dec) → ---
Comment 11•12 years ago
|
||
Pointer to Github pull-request
Comment 12•12 years ago
|
||
Comment on attachment 687652 [details] Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6780 1. Hide the keyboard when keyboard manager receives appwillclose event. 2. Remove redundant transition code in keyboard app. 3. I will add back the transitionend listener to keyboard manager so that the app resize would occur after the keyboard shows up.
Attachment #687652 -
Flags: review?(21)
Assignee | ||
Comment 13•12 years ago
|
||
Comment on attachment 687652 [details] Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6780 This looks really good to me when I tried it on the device. I'm not too familiar with the keyboard transition code, though.
Attachment #687652 -
Flags: feedback+
Assignee | ||
Comment 14•12 years ago
|
||
Pointer to Github pull-request
Assignee | ||
Comment 15•12 years ago
|
||
Comment on attachment 687692 [details] Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6780 Sorry, clicked the wrong button :(
Attachment #687692 -
Attachment is obsolete: true
Comment 16•12 years ago
|
||
Comment on attachment 687652 [details] Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6780 It also works very well on my device! r+.
Attachment #687652 -
Flags: review?(21) → review+
Comment 17•12 years ago
|
||
Comment on attachment 687652 [details] Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6780 Grr. This is not blocking anymore after has been a C2 bug and people has spent time on it.... Let's land what we have now.
Attachment #687652 -
Flags: approval-gaia-master+
Comment 18•12 years ago
|
||
https://github.com/mozilla-b2g/gaia/commit/e2814afbf12ef9500fe425a7746cb1cdd3cefc30
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 19•12 years ago
|
||
verified on Unagi 2012-12-03 gaia master : 5d150b2b10472478e8841730abd9ae9597595206 mozilla-beta : 1e56f66d7ee9
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•