Bug 107193

Summary:

Incorrect scrollable height during simplified layout

Product:

WebKit

Reporter:

Tony Chang <tony>

Component:

New Bugs

Assignee:

Tony Chang <tony>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

eric, hyatt, ojan.autocc, ojan, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch for landing	none

Tony Chang

Reported 2013-01-17 15:55:19 PST

Incorrect scrollable height during simplified layout

Attachments
Patch (4.60 KB, patch) 2013-01-17 16:31 PST, Tony Chang	no flags	Details Formatted Diff Diff
Patch (6.01 KB, patch) 2013-01-22 16:44 PST, Tony Chang	no flags	Details Formatted Diff Diff
Patch for landing (6.13 KB, patch) 2013-01-23 12:20 PST, Tony Chang	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Tony Chang

Comment 1 2013-01-17 16:31:51 PST

Created attachment 183312 [details] Patch

Tony Chang

Comment 2 2013-01-17 16:34:26 PST

I think this bug has been around since http://trac.webkit.org/changeset/73385 .

Ojan Vafai

Comment 3 2013-01-17 16:44:42 PST

Comment on attachment 183312 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183312&action=review > Source/WebCore/rendering/RenderBlock.cpp:2599 > + LayoutUnit logicalScrollHeight = style()->isHorizontalWritingMode() ? scrollHeight() : scrollWidth(); Should we add a helper function for this? I bet there are a bunch of places we do this already. > LayoutTests/fast/overflow/height-during-simplified-layout.html:17 > +shouldBe("document.getElementById('scrollable').scrollTop", "400"); > +shouldBe("document.getElementById('scrollable').scrollHeight", "600"); should we extend check-layout.js to support these?

WebKit Review Bot

Comment 4 2013-01-18 10:03:35 PST

Comment on attachment 183312 [details] Patch Clearing flags on attachment: 183312 Committed r140171: <http://trac.webkit.org/changeset/140171>

WebKit Review Bot

Comment 5 2013-01-18 10:03:39 PST

All reviewed patches have been landed. Closing bug.

Tony Chang

Comment 6 2013-01-22 11:05:10 PST

This caused the regression in bug 107572.

Tony Chang

Comment 7 2013-01-22 15:45:50 PST

Reverted r140171 for reason: Regressed scrollable region size in other cases. Committed r140482: <http://trac.webkit.org/changeset/140482>

Tony Chang

Comment 8 2013-01-22 15:48:40 PST

15:31 < dhyatt> i see 15:31 < dhyatt> the bug is it's using the clamped height 15:31 < dhyatt> computing of overflow needs to use the unclamped heigth 15:31 < dhyatt> height 15:32 < tony^work> right 15:32 < tony^work> but we don't have the unclamped height anymore 15:33 < dhyatt> could store it in the overflow 15:34 < dhyatt> so you could in theory put the intrinsic height into the overflow structure 15:34 < dhyatt> computeOverflow could even do it 15:34 < dhyatt> since it got passed it New patch with this approach coming soon.

Tony Chang

Comment 9 2013-01-22 16:44:46 PST

Created attachment 184075 [details] Patch

Dave Hyatt

Comment 10 2013-01-23 12:13:03 PST

Comment on attachment 184075 [details] Patch r=me, although I'd comment the code a bit to make it clear what you're doing.

Tony Chang

Comment 11 2013-01-23 12:20:55 PST

Created attachment 184280 [details] Patch for landing

WebKit Review Bot

Comment 12 2013-01-23 12:55:38 PST

Comment on attachment 184280 [details] Patch for landing Clearing flags on attachment: 184280 Committed r140576: <http://trac.webkit.org/changeset/140576>

WebKit Review Bot

Comment 13 2013-01-23 12:55:42 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.