1094066 - APP-CANCELLED key should handled by app first then fall back to system app

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Tzu-Lin Huang [:dwi2][:tzhuang]]

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.

Comment 1

[Tzu-Lin Huang [:dwi2][:tzhuang]]

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.

Comment 2

[Tzu-Lin Huang [:dwi2][:tzhuang]]

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.

Comment 3

[Tzu-Lin Huang [:dwi2][:tzhuang]]

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/

Comment 4

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Attachment: pull request

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.

Comment 5

[John Hu [:johnhu][:johu][:醬糊小弟]]

Comment on attachment 8529465 [details] [review]
pull request

Thanks for this patch. It looks good to me.

Comment 6

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Thanks,

Landed on master
https://github.com/mozilla-b2g/gaia/commit/677f5b2f87dccb57d8c41134586bf4d55ea4a53f