Bug 91299

Summary: Paginated views should restrict available height to column height
Product: WebKit Reporter: Beth Dakin <bdakin>
Component: Layout and RenderingAssignee: Beth Dakin <bdakin>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, dglazkov, eric, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
mitz: review-
Patch with test
webkit.review.bot: commit-queue-
Archive of layout-test-results from gce-cr-linux-07
none
Patch mitz: review+

Beth Dakin
Reported Saturday, July 14, 2012 12:48:56 AM UTC
Paginated views should restrict available height to column height. <rdar://problem/11152108>
Attachments
Patch (3.70 KB, patch)
2012-07-13 16:53 PDT, Beth Dakin
mitz: review-
Patch with test (30.20 KB, patch)
2012-07-13 18:30 PDT, Beth Dakin
webkit.review.bot: commit-queue-
Archive of layout-test-results from gce-cr-linux-07 (545.39 KB, application/zip)
2012-07-13 19:30 PDT, WebKit Review Bot
no flags
Patch (30.28 KB, patch)
2012-07-16 13:53 PDT, Beth Dakin
mitz: review+
Beth Dakin
Comment 1 Saturday, July 14, 2012 12:53:18 AM UTC
mitz
Comment 2 Saturday, July 14, 2012 1:09:03 AM UTC
Comment on attachment 152376 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152376&action=review > Source/WebCore/ChangeLog:9 > + You should be able to make a layout test for this, using window.internals.pagination. > Source/WebCore/rendering/RenderBlock.cpp:5023 > + if (isRenderView() && hasColumns()) > + return columnInfo()->columnHeight(); Why can’t RenderView be the one overriding this function? If we’re not a RenderView, isRenderView() is going to return false anyway. > Source/WebCore/rendering/RenderBlock.h:199 > + virtual LayoutUnit availableLogicalHeight() const; This override (though I think it will go in RenderView.h) should be annotated with the OVERRIDE macro.
Beth Dakin
Comment 3 Saturday, July 14, 2012 2:30:47 AM UTC
Created attachment 152393 [details] Patch with test
WebKit Review Bot
Comment 4 Saturday, July 14, 2012 3:30:55 AM UTC
Comment on attachment 152393 [details] Patch with test Attachment 152393 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13243301 New failing tests: fast/multicol/shrink-to-column-height-for-pagination.html
WebKit Review Bot
Comment 5 Saturday, July 14, 2012 3:30:58 AM UTC
Created attachment 152399 [details] Archive of layout-test-results from gce-cr-linux-07 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-07 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
mitz
Comment 6 Saturday, July 14, 2012 5:49:52 AM UTC
Comment on attachment 152393 [details] Patch with test View in context: https://bugs.webkit.org/attachment.cgi?id=152393&action=review > Source/WebCore/ChangeLog:25 > + New window.internals function allows setting the pagination with a pageLength. Since this is a JavaScript API, you can just add pageLength as an optional parameter to the existing function. Existing tests that don’t pass anything will behave as if they’d passed 0, I think, which is fine because 0 is the default value anyway.
Beth Dakin
Comment 7 Monday, July 16, 2012 9:53:51 PM UTC
mitz
Comment 8 Monday, July 16, 2012 9:59:06 PM UTC
Comment on attachment 152605 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152605&action=review > Source/WebCore/rendering/RenderView.cpp:111 > + Extra empty line.
Beth Dakin
Comment 9 Monday, July 16, 2012 10:07:38 PM UTC
Note You need to log in before you can comment on or make changes to this bug.