730434 - chrome/content/shell.js: remove 'home' event and other non-standard event types

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[David Flanagan [:djf]]

Attachment: patch to b2g/chrome/content/shell.js

chrome/content/shell.js currently sends a 'home' event when the Home key is pressed.

This patch changes it so that a 'home' event is sent on key up after a short press, and a 'switcher' event is sent if the home button is held down for 1 second.

This is needed to resolve: https://github.com/andreasgal/gaia/issues/642

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Why do you want to do such code here? This code can live on the Gaia side, right? (This behavior and the switcher event sound really specific to Gaia)

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 600515 [details] [diff] [review]
patch to b2g/chrome/content/shell.js

I agree with Vivien.

Comment 3

[David Flanagan [:djf]]

Vivien,

The code has to be here because shell.js currently sends the 'home' event on keypress.  I.e. it doesn't wait for the button to be released.  But for the new behavior we want in Gaia, we need it to send the home event when the button is released. 

Or we need to just get rid of the home event completely and do everything with keyup and keydown events in Gaia.  Either way, changes are needed here.

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

The home HW button should behave like a key.  If it doesn't we should fix that.

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

David, 

let's replace the 'sepcial' key events at http://mxr.mozilla.org/mozilla-central/source/widget/gonk/nsAppShell.cpp#252 y normal key events and let the content (Gaia) handle them normally.

Comment 6

[David Flanagan [:djf]]

Vivien,

nsAppShell.cpp already sends the Home key as a regualar key event.  It looks like menu, search and volume up and volume down are sent as "special" key events.

It is the shell.js file that I'm patching that turns these regular key events into a special, non-standard "home" event. 

I assumed that there was some good reason for that "home" event to be there in shell.js, which is why I modified the file to add a new event type.  But I agree that if we can just remove "home" events from shell.js, then we can handle the short press vs. long press distinction in content rather than chrome.

I assume those other key events are "special" because there is not an existing key code that they can use, but I don't know.  In any case, if they need to be changed in nsAppShell.cpp, that is a different bug, I think.

Comment 7

[David Flanagan [:djf]]

I've changed the title of the bug to match the discussion here, and have marked the attached patch as obsolete.

Comment 8

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

The patch remove all non-standards events from b2g/ (except showime/hideime but this is the scope of the mozKeyboard API).

There is some black magic to redirect key events to the homescreen if they are fired on an an other iframes and if they have not been cancelled.
The magic also always notify the homescreen of a keydown/press/up on the home button to ensure nobody prevent the user to go back to the homescreen.

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.2

This version sounds easier to understand.

Comment 10

[[:fabrice] Fabrice Desré]

Comment on attachment 601551 [details] [diff] [review]
Patch v0.2

Review of attachment 601551 [details] [diff] [review]:
-----------------------------------------------------------------

\o/ for removing commandUtils.js

Comment 11

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bef026927a58

Comment 12

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/bef026927a58