1001243 - Hitting the right edge of the toolbar in editing mode may fall through to the tabs button on devices without animations

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Turns out there is a difference between padding and margins on ImageViews! On certain devices, you can tap through the margins to the Views underneath them (hitting those buttons instead). In our case, this is tapping through the cancel button to the open tabs tray button underneath.

This is only a problem on devices without animations, where the tabs button does not get translated off screen. This is easily reproducible on an Xperia device I have but difficult on a Motorola Droid device (both pre-ICS).

Options:
* Replace the toolbar ImageView margins with padding
* setVisibility(View.INVISIBLE/GONE) on the menu items

We set a translation on the toolbar views [1], which I assume is not getting translated. So, we can *actually* translate the views off-screen if we wanted to too, but we should investigate why we bothered to try to do that in the first place (performance?).

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/toolbar/BrowserToolbar.java?rev=2ca6c9728ed9#1009

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I can confirm that this is reproducable on 30 with the go button (though it's much more difficult to do). I feel it's pretty insignificant so I'm going to WONTFIX 30 and below.

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Actually, I suppose I never tested 29 and lower.

Comment 3

[Aaron Train [:aaronmt]]

Does this look bad enough to track 31?

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Disable tabs button when in editing mode.

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Aaron Train [:aaronmt] from comment #3)
> Does this look bad enough to track 31?

It's really bad on the Xperia, and it's impossible to say how many devices are affected so yes, I think we should uplift.

The patch in comment 4 is a quick and easy solution. I tried to play around and fix some potential efficiency issues while I was at it, but it ended up not being worth my time - bug 1009250.

Comment 6

[Brian Nicholson (:bnicholson)]

Comment on attachment 8421306 [details] [diff] [review]
Disable tabs button when in editing mode.

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

Wouldn't it be preferable to replace the margins with the padding so that the hit area is as large as it should be? Changing the enabled state of the behind views seems hacky.

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Disable tabs button when in editing mode.

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8421360 [details] [diff] [review]
Disable tabs button when in editing mode.

Unintentional upload.

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Disable tabs button when in editing mode.

As discussed on IRC, changing the margins to padding does not fix this issue on the Droid so here is an updated patch also disabling the menuButton.

Comment 10

[Brian Nicholson (:bnicholson)]

Comment on attachment 8421372 [details] [diff] [review]
Disable tabs button when in editing mode.

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

I'd prefer to fix this in the foreground somehow instead of changing the enabled state of the behind views, which seems fragile. But I don't have any better suggestions for now.

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/33790bff0cdf

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8421372 [details] [diff] [review]
Disable tabs button when in editing mode.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  Pre-existing but became prevalent in bug 965548.

User impact if declined:
  Users on pre-ICS phones may unexpectedly open the tabs tray by hitting the not-visible tabs button when tapping the cancel button of the toolbar when trying to exit the awesomescreen

Testing completed (on m-c, etc.):
  Tested locally.
 
Risk to taking this patch (and alternatives if risky):
  Low - these are the only entry points to the awesomescreen and we're disabling/enabling two buttons that are drawn over at either point - it's hard to mess up, though something can always be overlooked.

String or IDL/UUID changes made by this patch: None

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/33790bff0cdf

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/cc69ae439121