Closed Bug 1148762 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox Graveyard :: Reading List, defect)

38 Branch
x86
macOS
defect
Not set

Tracking

(firefox38 verified, firefox39 fixed, firefox40 verified)

VERIFIED FIXED
Firefox 40
Tracking Status
firefox38 --- verified
firefox39 --- fixed
firefox40 --- verified

People

(Reporter: aleth, Assigned: bwinton)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

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.
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?
Flags: needinfo?(bmcbride)
Duplicate of this bug: 1149062
Blocks: 1148240
Summary: Scroll bar flashes when opening reading list sidebar → Reading List not full but scrollable for 1px, causes scroll bar to flash when opening reading list sidebar
(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.
Flags: needinfo?(bmcbride)
blake, can you take a look at this one?
Attached patch Okay, let's try this. (obsolete) — Splinter Review
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.)
Assignee: nobody → bwinton
Attachment #8588619 - Flags: review?(florian)
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.
Attached patch The next version of the patch. (obsolete) — Splinter Review
Sounds good.  Did you want to steal the review, Markus?
Attachment #8588619 - Attachment is obsolete: true
Attachment #8588619 - Flags: review?(florian)
Attachment #8588633 - Flags: review?(florian)
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.
Attachment #8588633 - Flags: review?(florian) → review+
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
Attachment #8588633 - Attachment is obsolete: true
Attachment #8588646 - Flags: review+
Attachment #8588646 - Flags: approval-mozilla-beta?
Attachment #8588646 - Flags: approval-mozilla-aurora?
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e09b166b8a7e
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 40
Comment on attachment 8588646 [details] [diff] [review]
The final version, ready for checkin.

Should be in 38 beta 3.
Attachment #8588646 - Flags: approval-mozilla-beta?
Attachment #8588646 - Flags: approval-mozilla-beta+
Attachment #8588646 - Flags: approval-mozilla-aurora?
Attachment #8588646 - Flags: approval-mozilla-aurora+
Duplicate of this bug: 1150601
QA Contact: andrei.vaida
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
Status: RESOLVED → VERIFIED
Removing qe-verify flag as verification on Firefox 38 Beta and 40 Nightly should suffice.
Flags: qe-verify+
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.