Bug 21462 – Overflow auto in div element shows obsolete scrollbars when content size is reduced

• Component: Layout and Rendering
• Status: RESOLVED DUPLICATE

[Matti Tahvonen]

When content becomes small enough to fit into container, scrollbars are not removed.

Reduced test case in URL field will show the case clearly (works in FF, IE, Opera. Bugs in Safari (both mac & win) and also in Chrome)

[Artur Signell]

Confirming the bug in latest nightly build r38492/windows. This has been bugging me also for some time.

[Artur Signell]

The problem seems to be that to RenderLayer::computeScrollDimensions uses clientHeight/clientWidth from the parent DIV to determine the amount of available space for the child DIV. ClientHeight/Width does not include the scrollbar sizes and thus it is wrongly determined that the child DIV will not fit inside the parent without scrollbars. I'm not sure if this should be fixed in computeScrollDimensions like done in the patch or if the scrollbars should be removed before calling computeScrollDimensions (called through RenderLayer::updateScrollInfoAfterLayout)

[Artur Signell]

Created attachment 26022 [details]
Patch for taking existing scrollbars into account

[Matti Tahvonen]

Hi!

Somebody with decent powers, please confirm this bug. This bugzilla seems to be so full that it is impossible to get tickets handled without it.

[Artur Signell]

Created attachment 26409 [details]
Updated patch including a webkit test

Contains patch and a test case which fails with trunk version and succeeds when the patch is used.

[Darin Adler]

The patch needs a change log entry too.

Please see http://webkit.org/coding/contributing.html for details.

[Artur Signell]

Created attachment 26424 [details]
Patch + test case + changelog entry

[Artur Signell]

Bug report, test case, test for webkit, suggested patch. What more can one do to get a bug fixed?

[Eric Seidel (no email)]

Created attachment 30184 [details]
A (slightly) cleaned up version of the test case

[Eric Seidel (no email)]

Comment on attachment 26424 [details]
Patch + test case + changelog entry

This patch uses tabs (which will cause the commit to fail.  This patch also adds extra { } which violate the style guidelines:
http://webkit.org/coding/coding-style.html

I cleaned up the test case slightly to use the "blue means you need to look at the test case to understand the results; green means pass and red means fail" system.

Otherwise, the patch looks sane to me.  Hyatt really needs to weigh in here though.  Ideally you'd update the patch to fix the style issues.  Also I would probably have used // for the clientHeight/clientWidth comment and would have broken it into two lines.  But I think /* is OK too, I'd have to check the style guidelines to be sure.

[Eric Seidel (no email)]

Adding Hyatt for his comments on this patch.

[Eric Seidel (no email)]

Comment on attachment 26424 [details]
Patch + test case + changelog entry

Great bug to fix.  r- for the style issues.  Added Hyatt to the CC list so he can also rubber-stamp.  Please post a copy with fixed style.

http://webkit.org/coding/coding-style.html

[Matti Tahvonen]

Our "official workaround" for this bug in Vaadin (formerly IT Mill Toolkit) revealed an other scrollbar related issue. I filled a separate bug report for it: https://bugs.webkit.org/show_bug.cgi?id=26438

Some of you superusers could confirm that too.

BTW. Still eagerly waiting for the fix to land in webkit. I was bit disappointed not to see this in safari 4. We could have removed a lot of dirty code from our product.

[Artur Signell]

Created attachment 31351 [details]
Updated patch (coding style, against latest revision)

[Dave Hyatt]

Comment on attachment 31351 [details]
Updated patch (coding style, against latest revision)

This does not seem quite right to me. I think it will just cause other cases to be wrong.

[Matti Tahvonen]

Hi!

If the patch by Artur is not enough, what else can we do to help you to fix this issue?

I have just spent one and a half day fighting with bugs and regressions which are either directly caused by this bug or its workarounds. So I'm really motivated to help fixing this issue. I'd much rather focus on developing new features. It would also enhance our products "feeling" a bit as we would get rid of some flickering caused by our workarounds.

cheers,
matti

[SravanKumar S(:sravan)]

*** This bug has been marked as a duplicate of bug 71541 ***