Closed Bug 857217 Opened 7 years ago Closed 7 years ago

Focus often jumps to the LayerView while navigating toolbar elements on the Ouya

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 23
Tracking Status
firefox21 --- unaffected
firefox22 --- fixed
firefox23 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(2 files)

Turns out the controllers still send lots of near-zero motion events, so the code I added in bug 855471 gets hit too often. Need to throttle this a bit so that joystick events in the dead zones don't move focus.
Attached patch PatchSplinter Review
Attachment #732458 - Flags: review?(chrislord.net)
This is probably also a good idea. It doesn't do very much until bug 849847 is fixed though.
Attachment #732460 - Flags: review?(chrislord.net)
Comment on attachment 732458 [details] [diff] [review]
Patch

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

LGTM.

::: mobile/android/base/util/GamepadUtils.java
@@ +42,5 @@
> +    public static boolean isValueInDeadZone(MotionEvent event, int axis) {
> +        float value = event.getAxisValue(axis);
> +        // The 1e-2 here should really be range.getFlat() + range.getFuzz() (where range is
> +        // event.getDevice().getMotionRange(axis)), but the values those functions return
> +        // on the Ouya are zero so we're just hard-coding it for now.

Is this comment still true with the latest firmware btw? On my Ouya, I still see unintentional drift with this number, I wonder if, regardless of the first question, we should increase it a bit? I don't think it's super-useful to be able to scroll a page *really* slowly anyway, and even if it was, we should probably just change the velocity curve rather than allow such a large range of motion on the stick...

This is really another bug though, getting side-tracked.
Attachment #732458 - Flags: review?(chrislord.net) → review+
Comment on attachment 732460 [details] [diff] [review]
Don't focus layerview if about:home is visible

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

Also LGTM.
Attachment #732460 - Flags: review?(chrislord.net) → review+
(In reply to Chris Lord [:cwiiis] from comment #3)
> Is this comment still true with the latest firmware btw?

Yeah, I re-tested it as I wrote this patch and it seems to still be returning 0. Do you know where we can file bugs against Ouya? The JIRA credentials that I got from them don't seem to work any more.

> On my Ouya, I still
> see unintentional drift with this number, I wonder if, regardless of the
> first question, we should increase it a bit? I don't think it's super-useful
> to be able to scroll a page *really* slowly anyway, and even if it was, we
> should probably just change the velocity curve rather than allow such a
> large range of motion on the stick...
> 
> This is really another bug though, getting side-tracked.

Yeah, I wouldn't be opposed to doing this as part of the "acceleration" bug.

Inbound is closed, so marking as checkin-needed.
Keywords: checkin-needed
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #5)
> (In reply to Chris Lord [:cwiiis] from comment #3)
> > Is this comment still true with the latest firmware btw?
> 
> Yeah, I re-tested it as I wrote this patch and it seems to still be
> returning 0. Do you know where we can file bugs against Ouya? The JIRA
> credentials that I got from them don't seem to work any more.

Would be worth pinging Al Sutton on this (al@ouya.tv - he's Cc'd on the main Ouya bug)

> > On my Ouya, I still
> > see unintentional drift with this number, I wonder if, regardless of the
> > first question, we should increase it a bit? I don't think it's super-useful
> > to be able to scroll a page *really* slowly anyway, and even if it was, we
> > should probably just change the velocity curve rather than allow such a
> > large range of motion on the stick...
> > 
> > This is really another bug though, getting side-tracked.
> 
> Yeah, I wouldn't be opposed to doing this as part of the "acceleration" bug.

Sounds good/sensible to me.

> Inbound is closed, so marking as checkin-needed.

Good thing I have the day off :)
Merged from try to m-c:

https://tbpl.mozilla.org/?tree=Try&rev=6348af4fe6aa

https://hg.mozilla.org/mozilla-central/rev/eb6993e036ff
https://hg.mozilla.org/mozilla-central/rev/6348af4fe6aa
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
Comment on attachment 732458 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 855471
User impact if declined: on the ouya focus will often disappear from the toolbar
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): mobile only, low risk
String or IDL/UUID changes made by this patch: none
Attachment #732458 - Flags: approval-mozilla-aurora?
Comment on attachment 732460 [details] [diff] [review]
Don't focus layerview if about:home is visible

[Approval Request Comment]
Ditto.
Attachment #732460 - Flags: approval-mozilla-aurora?
Attachment #732458 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #732460 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.