849845 - Map gamepad buttons to UI features

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Comment 1

[Chris Lord [:cwiiis]]

Attachment: Toggle chrome on Gamepad-Y button

This is blocked by bug 846772

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 723571 [details] [diff] [review]
Toggle chrome on Gamepad-Y button

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

Some nits but looks ok otherwise.

::: mobile/android/base/BrowserApp.java
@@ +296,5 @@
> +            return false;
> +        }
> +
> +        Log.d(LOGTAG, "Key event from view " + v + ": " + event);
> +        if (keyCode == KeyEvent.KEYCODE_BUTTON_Y) {

I think we also want if ((event.getSource() & InputDevice.SOURCE_GAMEPAD) == InputDevice.SOURCE_GAMEPAD) here. Also remove log line.

@@ +414,5 @@
>              }
>          }
>  
> +        // Intercept key events for gamepad shortcuts
> +        mBrowserToolbar.getLayout().setOnKeyListener(this);

You can use actionBar instead of mBrowserToolbar.getLayout() here. Also probably better to move this line up to near were actionBar is defined.

Comment 3

[Chris Lord [:cwiiis]]

Review comments addressed and dependency on bug 846772 removed. Pushed to inbound:

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

Adding leave-open to whiteboard.

Comment 4

[Chris Lord [:cwiiis]]

Attachment: Map gamepad L1/R1 to back/forward

Does what it says on the tin. Also rearranged the key callback function as I expect we're going to add a few more buttons/behaviours in there.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 723992 [details] [diff] [review]
Map gamepad L1/R1 to back/forward

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

::: mobile/android/base/BrowserApp.java
@@ +349,5 @@
> +                case KeyEvent.KEYCODE_BUTTON_R1:
> +                    // Go forward on R1
> +                    Tabs.getInstance().getSelectedTab().doForward();
> +                    return true;
> +                default:

I'd rather take out the "default:" since it has no body anyway.

Comment 6

[Chris Lord [:cwiiis]]

Comment addressed and pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/71d1bd406c7e

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Something in this push (along with bug 849958) was causing robocop-1/2 to timeout. Backed out.
https://hg.mozilla.org/integration/mozilla-inbound/rev/14c0d137f08e

Possibly of help - one of the runs shows this:
https://tbpl.mozilla.org/php/getParsedLog.php?id=20578472&tree=Mozilla-Inbound

INFO | automation.py | Application pid: 0
0 INFO SimpleTest START
1 INFO TEST-START | testHistoryTab
2 INFO TEST-PASS | testHistoryTab | Awesomebar URL typed properly - http://mochi.test:8888/tests/robocop/robocop_big_link.html should equal http://mochi.test:8888/tests/robocop/robocop_big_link.html
3 INFO TEST-PASS | testHistoryTab | Awesomebar URL typed properly - http://mochi.test:8888/tests/robocop/robocop_blank_01.html should equal http://mochi.test:8888/tests/robocop/robocop_blank_01.html
INFO | automation.py | Application ran for: 0:00:57.216669
INFO | automation.py | Reading PID log: /var/folders/jr/dj51k_ns4vq2yg4mcygb26gw0000gn/T/tmpRCOathpidlog
PROCESS-CRASH | java-exception | java.lang.NoSuchMethodError: android.view.KeyEvent.getSource	at org.mozilla.gecko.BrowserApp.onKey(BrowserApp.java:329)
WARNING | automationutils.processLeakLog() | refcount logging is off, so leaks can't be detected!

INFO | runtests.py | Running tests: end.

Note that the timeouts were always after testAllPagesTab.

Comment 9

[Chris Lord [:cwiiis]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #8)
> Something in this push (along with bug 849958) was causing robocop-1/2 to
> timeout. Backed out.
> https://hg.mozilla.org/integration/mozilla-inbound/rev/14c0d137f08e
> 
> Possibly of help - one of the runs shows this:
> https://tbpl.mozilla.org/php/getParsedLog.php?id=20578472&tree=Mozilla-
> Inbound
> 
> INFO | automation.py | Application pid: 0
> 0 INFO SimpleTest START
> 1 INFO TEST-START | testHistoryTab
> 2 INFO TEST-PASS | testHistoryTab | Awesomebar URL typed properly -
> http://mochi.test:8888/tests/robocop/robocop_big_link.html should equal
> http://mochi.test:8888/tests/robocop/robocop_big_link.html
> 3 INFO TEST-PASS | testHistoryTab | Awesomebar URL typed properly -
> http://mochi.test:8888/tests/robocop/robocop_blank_01.html should equal
> http://mochi.test:8888/tests/robocop/robocop_blank_01.html
> INFO | automation.py | Application ran for: 0:00:57.216669
> INFO | automation.py | Reading PID log:
> /var/folders/jr/dj51k_ns4vq2yg4mcygb26gw0000gn/T/tmpRCOathpidlog
> PROCESS-CRASH | java-exception | java.lang.NoSuchMethodError:
> android.view.KeyEvent.getSource	at
> org.mozilla.gecko.BrowserApp.onKey(BrowserApp.java:329)
> WARNING | automationutils.processLeakLog() | refcount logging is off, so
> leaks can't be detected!
> 
> INFO | runtests.py | Running tests: end.
> 
> Note that the timeouts were always after testAllPagesTab.

Whoops, so gamepad support was only added in API level 9 and we support 8 - I've added the relevant check, will re-push when the tree opens.

Comment 10

[Chris Lord [:cwiiis]]

Added API check, pushed again:

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

Comment 11

[Ed Morley [:emorley]]

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

Comment 12

[Kartikaya Gupta (email:[e-mail])]

I'm going to mark this bug closed. We can map other buttons in other bugs.