Bug 91299 – Paginated views should restrict available height to column height

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Beth Dakin]

Paginated views should restrict available height to column height.

<rdar://problem/11152108>

[Beth Dakin]

Created attachment 152376 [details]
Patch

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]

Created attachment 152393 [details]
Patch with test

[WebKit Review Bot]

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]

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

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]

Created attachment 152605 [details]
Patch

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]

Thanks Dan! Committed: http://trac.webkit.org/changeset/122761