1150251 - Tapping +/- buttons makes Aa move slightly side to side

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Reader View, defect)

Description

[:Margaret Leibovic]

I wonder if this is a regression. Teodora, have you seen this?

Comment 1

[Teodora Vermesan (:TeoVermesan)]

Yes, this is a regression. I don't see the bounce on the 31-03 build. But I can reproduce on the 01-04 build. 

pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8af276ab8636&tochange=0b88606f8fe7
It's Bug 1142528 - Tappable area is larger than the +/- buttons, overlaping "Aa" button

Comment 2

[:Margaret Leibovic]

Vivek, I'm hoping you can take this :)

I can also look into this if you're too busy with other things, let me know if you'd prefer that I take it.

Comment 3

[Vivek Balakrishnan[:vivek]]

Attachment: 1150251.patch

It seems there is a slight decrease in width when a button is in the pressed state. This is the reason why Aa button side to side when +/- button is pressed. Added padding to +/- button to solve this.

Comment 4

[:Margaret Leibovic]

Comment on attachment 8587645 [details] [diff] [review]
1150251.patch

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

Testing locally, when I tap on the "Aa", it now moves, so I don't think this is an acceptable fix.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Let's backout bug 1142528 from Fx38 and Fx39. Then we can fix this on Nightly.

Comment 6

[:Margaret Leibovic]

Backed out bug 1142528 from aurora/beta, so this is less time sensitive now.

Handing this back to Vivek.

Comment 7

[Vivek Balakrishnan[:vivek]]

Attachment: 1150251.patch

Added padding for Aa sample to avoid shake effect.

Comment 8

[:Margaret Leibovic]

Comment on attachment 8590501 [details] [diff] [review]
1150251.patch

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

This works well with my testing. However, I still don't totally understand what these padding changes are doing to change the behavior. Do you understand what's going on?

::: mobile/android/themes/core/aboutReader.css
@@ +400,5 @@
>    color: #000000;
> +  margin-left: 30px;
> +  margin-right: 30px;
> +  padding-left: 10px;
> +  padding-right: 10px;

Nit: Could you format these on one line each?

margin: 0 30px;
padding: 0 10px;

Comment 9

[Vivek Balakrishnan[:vivek]]

Attachment: 1150251.patch

Review nit corrected.
@Margaret: As I mention in irc, I don't clearly understand the reasons. I suspect it is due to some browser style for hover state for the buttons.

Comment 10

[:Margaret Leibovic]

Let's let this land on Nightly, and if there are no regressions, we can think about re-uplifting bug 1142528 along with this patch.

Comment 11

[:Margaret Leibovic]

https://hg.mozilla.org/integration/fx-team/rev/165ef2e74b13

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/165ef2e74b13

Comment 13

[Teodora Vermesan (:TeoVermesan)]

Verified as fixed using:
Device: Nexus 7 (Android 5.0)
Build: Firefox for Android 40.0a1 (2015-04-15)