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

RESOLVED FIXED in Firefox 31

Status

()

defect
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: mcomella, Assigned: mcomella)

Tracking

unspecified
Firefox 32
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox30 wontfix, firefox31+ fixed, firefox32 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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
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.
Summary: Hitting the toolbar cancel button will fall through to the tabs button on devices without animations → Hitting the right edge of the toolbar in editing mode may fall through to the tabs button on devices without animations
Actually, I suppose I never tested 29 and lower.
Does this look bad enough to track 31?
(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 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 on attachment 8421360 [details] [diff] [review]
Disable tabs button when in editing mode.

Unintentional upload.
Attachment #8421360 - Attachment is obsolete: true
Attachment #8421360 - Flags: review?(bnicholson)
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.
Attachment #8421372 - Flags: review?(bnicholson)
Attachment #8421306 - Attachment is obsolete: true
Attachment #8421306 - Flags: review?(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.
Attachment #8421372 - Flags: review?(bnicholson) → review+
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
Attachment #8421372 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/33790bff0cdf
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Attachment #8421372 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.