1272677 - Two parallel lines are displayed in "Aa" floating layout button in reader view

Status: VERIFIED FIXED

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

Description

[Teodora Vermesan (:TeoVermesan)]

Attachment: Screenshot from 2016-05-13 17:22:22.png

Steps to reproduce:
1. Open an article and enter reader view
2. Tap the "Aa" button

Actual results:
- Two parallel lines are displayed between "Aa" and "Auto" in the "Aa" floating layout button

Comment 1

[:Margaret Leibovic]

Is this a recent regression? I'm worried this could be caused by desktop reader view changes.

Comment 2

[:Gijs (he/him)]

(In reply to :Margaret Leibovic from comment #1)
> Is this a recent regression? I'm worried this could be caused by desktop
> reader view changes.

Maybe... I don't think there have been that many recently... at least, not that I would assume would affect layout, in the last week or two, that also haven't been uplifted. :-\

A regression window would be very helpful.

Comment 3

[Teodora Vermesan (:TeoVermesan)]

Regression:
10-05 not affected
11-05 affected

pushlog: 
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=043082cb7bd8490c60815f67fbd1f33323ad7663&tochange=674a552743785c28c75866969aad513bd8eaf6ae

Comment 4

[:Gijs (he/him)]

(In reply to Teodora Vermesan (:TeoVermesan) from comment #3)
> Regression:
> 10-05 not affected
> 11-05 affected
> 
> pushlog: 
> http://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=043082cb7bd8490c60815f67fbd1f33323ad7663&tochange=674a
> 552743785c28c75866969aad513bd8eaf6ae

Thanks! That's bug 1269521 then... though looking at the markup:

         </div>
         <hr></hr>
         <div id="content-width-buttons">
           <button id="content-width-minus" class="content-width-minus-button"/>
           <button id="content-width-plus" class="content-width-plus-button"/>
         </div>
         <hr></hr>
+        <div id="line-height-buttons">
+          <button id="line-height-minus" class="line-height-minus-button"/>
+          <button id="line-height-plus" class="line-height-plus-button"/>
+        </div>
+        <hr></hr>
         <div id="color-scheme-buttons"></div>

not sure why the added <hr>s are showing up on Android but the pre-existing ones don't. Margaret?

Comment 5

[:Margaret Leibovic]

(In reply to :Gijs Kruitbosch from comment #4)
> (In reply to Teodora Vermesan (:TeoVermesan) from comment #3)
> > Regression:
> > 10-05 not affected
> > 11-05 affected
> > 
> > pushlog: 
> > http://hg.mozilla.org/mozilla-central/
> > pushloghtml?fromchange=043082cb7bd8490c60815f67fbd1f33323ad7663&tochange=674a
> > 552743785c28c75866969aad513bd8eaf6ae
> 
> Thanks! That's bug 1269521 then... though looking at the markup:
> 
>          </div>
>          <hr></hr>
>          <div id="content-width-buttons">
>            <button id="content-width-minus"
> class="content-width-minus-button"/>
>            <button id="content-width-plus"
> class="content-width-plus-button"/>
>          </div>
>          <hr></hr>
> +        <div id="line-height-buttons">
> +          <button id="line-height-minus" class="line-height-minus-button"/>
> +          <button id="line-height-plus" class="line-height-plus-button"/>
> +        </div>
> +        <hr></hr>
>          <div id="color-scheme-buttons"></div>
> 
> not sure why the added <hr>s are showing up on Android but the pre-existing
> ones don't. Margaret?

I see we do have a style in place to hide these <hr>s:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/aboutReaderControls.css#130

Not sure why that's not being picked up...

Dan, can you own fixing this?

Comment 6

[Dan Huang[:danhuang]]

Attachment: MozReview Request: Bug 1272677 - Hide content width setting and line-height setting buttons in android's reader mode control panel. r?Margaret

Review commit: https://reviewboard.mozilla.org/r/55770/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/55770/

Comment 7

[:Margaret Leibovic]

Comment on attachment 8757238 [details]
MozReview Request: Bug 1272677 - Hide content width setting and line-height setting buttons in android's reader mode control panel. r?Margaret

https://reviewboard.mozilla.org/r/55770/#review52910

Comment 8

[Dan Huang[:danhuang]]

Thanks for the review.

Comment 9

[Pulsebot]

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

Comment 10

[Carsten Book [:Tomcat]]

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

Comment 11

[Teodora Vermesan (:TeoVermesan)]

Verified as fixed using:
Device: One A2001 (Android 5.1.1)
Build: Firefox for Android 49.0a1 (2016-06-01)