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: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Tony Chang
2013-01-17 15:55:19 PST
Created attachment 183312 [details]
Patch
I think this bug has been around since http://trac.webkit.org/changeset/73385 . 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? Comment on attachment 183312 [details] Patch Clearing flags on attachment: 183312 Committed r140171: <http://trac.webkit.org/changeset/140171> All reviewed patches have been landed. Closing bug. This caused the regression in bug 107572. Reverted r140171 for reason: Regressed scrollable region size in other cases. Committed r140482: <http://trac.webkit.org/changeset/140482> 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. Created attachment 184075 [details]
Patch
Comment on attachment 184075 [details]
Patch
r=me, although I'd comment the code a bit to make it clear what you're doing.
Created attachment 184280 [details]
Patch for landing
Comment on attachment 184280 [details] Patch for landing Clearing flags on attachment: 184280 Committed r140576: <http://trac.webkit.org/changeset/140576> All reviewed patches have been landed. Closing bug. |