416752 - Useless scrollbars on right and bottom of a <div> that uses overflow: auto or overflow: scroll

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[Jon Vaughan]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.12) Gecko/20080201 Firefox/2.0.0.12
Build Identifier: Firefox 2008020511

Screenshot: http://turnip.org.uk/ff3/firefox_testcase/screenshot_osx.png

First noticed on several SMF forums, I have reduced it for the test page at http://turnip.org.uk/ff3/firefox_testcase/bg.html

if 'overflow: ' is specified as anything other than auto or scroll, the problem does not occur.  It also does not occur on platforms other than Mac OS X or on Mac OSX version 2.0.0.x



Reproducible: Always

Steps to Reproduce:
1. Visit http://turnip.org.uk/ff3/firefox_testcase/bg.html

Actual Results:  
A large border is visible to the bottom and right of the text on the page.

Expected Results:  
not displayed the border

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

That "border" is the scroll wells/tracks.

Philippe, is this a dupe of that bug you filed about the extraneous scroll wells from the forum?

Comment 2

[Jay Moran (jaym/lazvon)]

I can confirm this as well. Annoying! It is definitely the Scroll Bars... even when no Scroll Bars are needed. No Up/Down arrows in the bars though. Visible even in the CC box here in BugZilla for me.

Comment 3

[philippe (part-time)]

(In reply to comment #1)
> That "border" is the scroll wells/tracks.
> 
> Philippe, is this a dupe of that bug you filed about the extraneous scroll
> wells from the forum?
> 
Nope :-(. That one (bug 412607) is already fixed.

It is another weird thing. I can only repro it if I have 'Verdana' and/or 'Tahoma' enabled.

@Jon Vaughan (comment 0)
Is that a recent regression ?

Comment 4

[philippe (part-time)]

Attachment: test case from reporter

Test case, slightly more minimised.

This seems to be triggered by certain values of line-height, and at certain font-sizes.
The top box in the testcase is the original; for the bottom box I made the line-height slightly larger: going from 1.3em to 1.34em is enough to 'kill' that dead scrollbar.

Comment 5

[Jay Moran (jaym/lazvon)]

Attachment: Screenshot of bugzilla CC box

This is on: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko/2008021113 Firefox/3.0b4pre

Should not be a scroll bar box on the right since it is not needed.

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (mac)

Here's a reduced testcase that reproduces the issue on mac.

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: testcase 3 (linux)

Here's a reduced testcase with slightly different heights that reproduces the bug on Linux.

Comment 8

[Daniel Holbert [:dholbert]]

Comment on attachment 302700 [details]
Screenshot of bugzilla CC box

The bugzilla CC box is not a problem.

It's supposed to have a scrollbar on it -- it's a <select> pane.  It also looks the same in FF2 as it in FF3, so it's not a regression.

Comment 9

[Daniel Holbert [:dholbert]]

Attachment: testcase 4 (windows + linux)

Here's another one like testcases 2 and 3, tweaked slightly to reproduce the bug on both Windows and Linux.

Comment 10

[Jon Vaughan]

One side effect of this bug is that after clicking the mouse in an affected <div>, the cursor and other keys no longer scroll the page (presumably trying to scroll the content of the <div> instead)

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: universal testcase

This is simpler and should show the bug on all platforms because it doesn't rely on line heights.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Here's what's happening with that testcase on Mac in nsHTMLScrollFrame::ReflowContents:

-- We try a layout with a vertical scrollbar (with or without a horizontal scrollbar doesn't matter). The vertical scrollbar has a preferred height of 30 pixels, so that's what we set our putative scrollport height to. That's enough for to hold the content including its vertical overflow (which is 16px high), so we decide that no vertical scrollbars are needed, here:
http://mxr.mozilla.org/seamonkey/source/layout/generic/nsGfxScrollFrame.cpp#399
So we reject this layout.

-- We try a layout with no vertical scrollbar (again, with or without a horizontal scrollbar doesn't matter). This time since there is no vertical scrollbar the putative scrollport height is set to the height of the scrolled content, which is 15 pixels. At line 399 this time we note that there is overflow (1 pixel), so wantVScrollbar is set to true. At line 404 we have an additional check that sets wantVScrollbar to false if the putative scrollport height is less than the scrollbar *minimum* height, but this check fails because the minimum height is only 10 pixels, so we conclude we do want a vertical scrollbar, so we reject this layout.

-- We give up because we can't find a consistent scrollbar setup, and fall back to showing both scrollbars.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

So what is the desired rendering of this testcase? I think the outer DIV should be 15 pixels high, with a vertical scrollbar letting you scroll down to see the 1-pixel overflow. To achieve that we should change TryLayout so instead of increasing the scrollport height to at least the *preferred* height of the scrollbar, we should increase the scrollpart height to at least the *minimum* height of the scorllbar.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We need to be careful not to regress bug 292312. I think we'll be OK if we just change the setting of the putative scrollPortSize to use the minimum vertical scrollbar height and minimum horizontal scrollbar width, but continue to use the preferred vertical scrollbar width and preferred horizontal scrollbar height elsewhere.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Fix as described above, plus testcase

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I ran reftests, they passed (except for a few tests which always fail on my box due to subpixel issues).

Comment 17

[Jay Moran (jaym/lazvon)]

Applied your patch to a checkout about 1.5 hours ago... fixed halfway for me. It gets rid of the horizontal scroll bars for, but I still have the vertical scroll bars which are every bit as annoying to me.

I tested (same results with all, horizontal gone, vertical still there):
1) Above tests, including the new universal one. 
2) SMF Forums.
3) Google Docs Spreadsheet, editing cells.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think the vertical scrollbars are necessary. These elements do actually have vertical overflow.

Comment 19

[Jay Moran (jaym/lazvon)]

Attachment: Screenshot of SMF Forums Un-needed Vertical Bars

I disagree that the vertical bars are showing up because they are needed. I've attached a screenshot of them showing up in SMF forums where they aren't needed. Also, even with the attached test cases, why should there be a "placeholder" for a vertical scroll bar, but no actual up/down buttons to move up and down with. If the vertical scroll bar is "needed" then there should be a way to use it.

Is there a way to do something along the lines of "If not drawing up/down arrows, then don't display vertical scroll bar".

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The scrollbars would go away if we increased the minimum scrollbar size on Mac. Josh, do you know why the XUL minimum scrollbar size on Mac is only 10 pixels? That should probably have its own bug though.

The testcases in this bug definitely have vertical overflow. The overflow isn't visible but the CSS boxes overflow and we should be able to scroll to them as per spec. For example if you set the line-height to 2px you should definitely be able to scroll down to see the text on the last line. I think the real SMF forums have basically the same issue. Looking here:
http://aplanding.com/smf/index.php?PHPSESSID=vcbt6ecsp1vituq8urt9mq1p90&topic=973.msg5975;topicseen#new
on my Mac build, the last line is close to the bottom and the descenders on the g and j are actually cropped by one pixel. The issues would go away if they increased the line-height a bit more or wrapped the whole comment in an overflow:hidden block.

Comment 21

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

There are a lot of places in nsGfxScrollFrame that use both min and pref sizes of scrollbars.  Is it reasonable to assume that the min and pref sizes of a scrollbar are equal in the direction transverse to the direction that scrollbar scrolls?  If so, should we assert that?  If we make that assumption, and then use only the min size in the direction of scrolling (except for pref size calculation itself, perhaps), then this begins to make sense.

... I think that's pretty much where this patch gets us, with the exception of nsXULScrollFrame::AddRemoveScrollbar, and with the exception of actually being consistent about which of pref or min size we actually use in the scrolling direction.

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 304181 [details] [diff] [review]
fix

Anyway -- I'm ok with this, but if you agree with my previous comment, maybe file some followup bugs?

r+sr=dbaron

Comment 23

[Robert O'Callahan (:roc) (email my personal email if necessary)]

> Is it reasonable to assume that the min and pref sizes of a scrollbar are equal > in the direction transverse to the direction that scrollbar
> scrolls?

No. Bug 292312 was a regression caused by that assumption being violated ... the min-width for GTK scrollbars was too narrow.

Now, perhaps we should have a bug on making the assumption true on all platforms.

Comment 24

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Checked in. Do you agree that we should make scrollbar min and pref sizes the same on all platforms?

Comment 25

[Robert O'Callahan (:roc) (email my personal email if necessary)]

oops, marking fixed

Comment 26

[Jon Vaughan]

Re-opening this as it doesn't seem to be to be resolved.

I still have the same issue as Jay Moran does in comment #19.  It seems only the horizontal scrollbars have been removed.

Comment 27

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Please file a new bug for the vertical scrollbars. It probably won't receive blocking status.

Comment 28

[Jon Vaughan]

Why should a horizontal scrollbar get blocking status, but not vertical?  

It is still a bug which fails your "universal testcase", looks bad and prevents page scrolling with the cursor keys.  

I could maybe understand if the scrollbars were actually usable, but they aren't.

Comment 29

[Jay Moran (jaym/lazvon)]

I have to agree with Jon here, not sure I understand why horizontal would get blocking status and vertical wouldn't. Is it just a timing thing being so close to release? If a decision can be made to NOT put the UP/DOWN arrows on the scroll bar, would be nice to not show a scroll bar at all. Just would like to see the functionality in FFx2 restored. I've included two screen shots... FFx2 without the vertical scroll bars ... FFx3 with them.

Comment 30

[Jay Moran (jaym/lazvon)]

Attachment: FFx2 Screenshot - no vert scrollbars

Comment 31

[Jay Moran (jaym/lazvon)]

Attachment: FFx3 Screenshot - empty vert scrollbars where they shouldn't be

Comment 32

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Showing the horizontal scrollbar was a plain bug. There was no need to show it.

Showing the vertical scrollbar is not really a bug. We have an overflow:auto element with vertical overflow, so, we show the scrollbar.

Now in this case (on Mac at least) we probably shouldn't show the scrollbar because it's too small to work. That's a bug which we could fix for FF3 but we may not have time. But if we fixed that, if the overflow:auto element was tall enough to contain a working vertical scrollbar, we would definitely want to show it, so perhaps you guys would not be satisfied anyway.

Note that in FF3 font metrics and certain other text layout details have changed, which is maybe why FF2 doesn't have vertical overflow in this case. But if you changed the line-height in the testcase to something smaller I bet FF2 would show the same "bug".

Comment 33

[Ken Palmer]

This is a problem we are experiencing with the default Theme for Firefox on the Mac.  This problem can be seen in the Microsoft SQL Reporting Services ReportViewer control, which renders a multi-select dropdown list as a table within a div with an overflow:auto style.

Our team discovered that the problem was related directly to the default theme, and not to the browser itself.  The complete post of our findings appears at this link:
http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=2981260&SiteID=1&mode=1

What was notable to us is that the Nature / Metal For Firefox theme worked.  It properly displayed the scrollbars.  The default theme for PC worked.  But the default theme for Mac did not work.

Comment 34

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Ken: this bug is really about scrollbars appearing when they're not wanted. Your issue seems to be something else. Please file a new bug, especially if you can provide a reduced test case (something that's just plain HTML), and especially if this is a bug that wasn't in Firefox 2 (i.e. a regression in Firefox 3). It's not clear to me from your messages whether you were seeing this in Firefox 2 or Firefox 3.

Comment 35

[Dan Mellem]

Attachment: Scrollbar with ABBR

Comment 36

[Dan Mellem]

I'm seeing this on SeaMonkey 2.16 (Build identifier: 20130217195808)

Testcase 4 still shows a superfluous scrollbar.
I created a new testcase that has an ABBR, and it adds a scrollbar for the underline.

I'm seeing this for both SeaMonkey 2.16 and Firefox 19.0.

Comment 37

[Daniel Holbert [:dholbert]]

Dan: Could you file a new bug on the issue you're seeing? This bug has been closed for ~5 years, and it had a patch checked in to fix it, and at least roc's attached "universal testcase" appears to still be fixed in trunk, so it hasn't completely regressed. (though I can confirm that the testcase you attached does have a scrollbar)

Comment 38

[Dan Mellem]

No problem. Bug 847675.