Closed Bug 1094066 Opened 5 years ago Closed 5 years ago
APP-CANCELLED key should handled by app first then fall back to system app
There is a known issue after bug 1014418 lands. According to https://bugzilla.mozilla.org/show_bug.cgi?id=1014418#c20 and https://bugzilla.mozilla.org/show_bug.cgi?id=989198#c194, the actual key event sequence of the patch in bug 989198 should be: 1. mozbrowserbeforekeydown event in embedder frame (i.e. System app) 2. keydown event in embedder frame (*) 3. keydown event in embedded frame 4. mozbrowserafterkeydown event in embedder frame (*): Sequence number 2 is not in our assumption when we start implementing this mechanism. The sequence number 2 above might cause newly implemented browser_key_event_manager in System app accidentally intercepts and processes key event even before other app does. This would make 'app-cancelled' scenario failed. However this has no effect on all current apps. Because no app directly listens to KeyEvent of VolumeUp/VolumeDown now.
To post my findings here: TL;DR, keydown/keyup events for System app when focus is on System and keydown/keyup events described in https://bugzilla.mozilla.org/show_bug.cgi?id=1094066#c0 have different |event.target|. We could use |event.target| to distinguish them. In current implementation in System app, we listen to keydown/keyup for all keys except 'Home' and 'Power' because we treat them as 'APP-CANCELLED' scenario keys. (See https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/browser_key_event_manager.js#L65 and https://wiki.mozilla.org/WebAPI/BrowserAPI/KeyboardEvent#Part_2_-_Key_event_handling_customization ) However, according to https://bugzilla.mozilla.org/show_bug.cgi?id=1094066#c0 we might intercept and process 'VolumeDown' and 'VolumeUp' keys in system app earlier than foreground apps have any chance to process it. I found out that when focus is on System app, target of keydown/keyup event is element in system app (usually body element). But target of keydown/keyup event described in https://bugzilla.mozilla.org/show_bug.cgi?id=1094066#c0 are always mozbrowser iframe of the focused app. We probably could utilize event.target to decides whether we should process keydown/keyup event in System app.
Summary: VolumeUp and VolumeDown key should handled by app first then fall back to system app → APP-CANCELLED key should handled by app first then fall back to system app
Change the title because this should apply to all APP-CANCELLED keys, not just VolumeUp and VolumeDown. The definition of APP-CANCELLED keys are at https://wiki.mozilla.org/WebAPI/BrowserAPI/KeyboardEvent#Dispatch_KeyboardEvent_across_BrowserElements VolumeUp and VolumeDown are also defined as APP-CANCELLED keys.
Status: NEW → ASSIGNED
Basically, this patch is just the same as the one in https://bugzilla.mozilla.org/show_bug.cgi?id=1103339 But we'll patch it in tv_apps/
Hi John, Could you please review the patch? Thanks With this patch, we could let embedded app to handle APP-CANCELLED keys first, then fallback to system app if key event is not processed.
Attachment #8529465 - Flags: review?(im)
Comment on attachment 8529465 [details] [review] pull request Thanks for this patch. It looks good to me.
Attachment #8529465 - Flags: review?(im) → review+
Thanks, Landed on master https://github.com/mozilla-b2g/gaia/commit/677f5b2f87dccb57d8c41134586bf4d55ea4a53f
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.