1148762 - Reading List not full but scrollable for 1px, causes scroll bar to flash when opening reading list sidebar

Status: VERIFIED FIXED

(Firefox Graveyard :: Reading List, defect)

Description

[aleth [:aleth]]

STR 
Open a page in reader view
Click on the "open sidebar" button

A vertical scrollbar stretching over the whole height of the page flashes.

Comment 1

[Markus Stange [:mstange]]

This is caused by the height:100% on the body and the border-top on the html element of the document in the sidebar browser. It doesn't seem like the heights are necessary. Are they?

Oh, maybe it's for putting the horizontal scrollbar on the bottom. Why are the overflow-x and overflow-y scrollbars on different elements?

Comment 3

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Markus Stange [:mstange] from comment #1)
> It doesn't seem like the heights are necessary. Are they?

Er, hm. I can't remember why I added that... but doesn't seem to be needed, based on some brief testing.

> Oh, maybe it's for putting the horizontal scrollbar on the bottom. Why are
> the overflow-x and overflow-y scrollbars on different elements?

Hmm, I don't think we ever want a horizontal scrollbar. We only need #list to scroll vertically when it's overflowing.

Comment 4

[Bryan Clark (DevTools PM) [:clarkbw]]

blake, can you take a look at this one?

Comment 5

[Blake Winton (:bwinton) (:☕️)]

Attachment: Okay, let's try this.

Like Unfocused said, removing the height seems to fix it with no ill effects, so let's do that.  (Adding box-sizing: border-box also fixed it, but this seems simpler.)

Comment 6

[Markus Stange [:mstange]]

I think you should also remove the overflow-x: auto from the other element, so that you don't have a horizontal scrollbar at the last list item.

Comment 7

[Blake Winton (:bwinton) (:☕️)]

Attachment: The next version of the patch.

Sounds good.  Did you want to steal the review, Markus?

Comment 8

[Markus Stange [:mstange]]

Comment on attachment 8588633 [details] [diff] [review]
The next version of the patch.

Sure. You can probably also remove the height: 100%; from #list, unless you see a reason for it to be there.

Comment 9

[Blake Winton (:bwinton) (:☕️)]

Attachment: The final version, ready for checkin.

Done and done.

Approval Request Comment
[Feature/regressing bug #]: readinglist
[User impact if declined]: A scrollbar will appear when the user opens the reading list, making it harder to click the remove buttons until it disappears.
[Describe test coverage new/current, TreeHerder]: Manual testing on OS X.
[Risks and why]: Minimal risk, due to a css-only change.
[String/UUID change made/needed]: None

Comment 10

[(PTO June 19-July 7) Ryan VanderMeulen]

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

Comment 11

[Ekanan Ketunuti]

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

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8588646 [details] [diff] [review]
The final version, ready for checkin.

Should be in 38 beta 3.

Comment 13

[(PTO June 19-July 7) Ryan VanderMeulen]

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

Comment 14

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-beta/rev/de78faf679e7

Comment 16

[Ada [:adalucinet]]

Reproduced with Nightly from 2015-03-28.
Verified as fixed with Firefox 38 beta 6 (Build ID: 20150420134330) and latest 40.0a1 (2015-04-28) on Mac OSX 10.9.5

Comment 17

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Removing qe-verify flag as verification on Firefox 38 Beta and 40 Nightly should suffice.