Bug 131811 – [New Multicolumn] Pagination direction not being honored (e.g., BottomToTop-tb.html)

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Dave Hyatt]

This test seems to be ignoring the pagination direction with the new multi-column code. The writing mode is fine, but the bottom-to-top pagination direction is being ignored.

[Dave Hyatt]

Same bug occurs with LeftToRight-rl.html.

[Dave Hyatt]

RightToLeft-lr has the problem also.

[Dave Hyatt]

TopToBottom-bt has the problem too.

[Dave Hyatt]

Created attachment 229610 [details]
Patch

[Dave Hyatt]

Created attachment 229612 [details]
Patch

[Dean Jackson]

Comment on attachment 229612 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=229612&action=review

> LayoutTests/fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb-expected.html:25
> +<div class="box" onclick="this.classList.toggle('changed')"></div>

Did you mean to leave in the onclick and .box.changed style rules?

[Dave Hyatt]

Fixed in r167478

[Simon Fraser (smfr)]

4 tests broken and the new test crashes:
http://build.webkit.org/results/Apple%20Mavericks%20Release%20WK2%20(Tests)/r167479%20(5010)/results.html

[WebKit Commit Bot]

Re-opened since this is blocked by bug 131840

[Alexey Proskuryakov]

Rolled out in <https://trac.webkit.org/r167483>. Simon, it would be helpful if you could roll out when you noticed this, to avoid having the tests fail for hours.

[Radar WebKit Bug Importer]

<rdar://problem/16656269>

[Dave Hyatt]

Well that's weird. Completely passes on my machine still.

[Alexey Proskuryakov]

The results link posted above didn't have a crash log. Here is one from a debug build (release one seems to be pretty much the same, but I don't have a link):

http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK2%20(Tests)/r167479%20(4041)/fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb-crash-log.txt

ASSERTION FAILED: hasColumns()
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/rendering/RenderBlock.cpp(3390) : unsigned int WebCore::RenderBlock::columnCount(WebCore::ColumnInfo *) const
1   0x110df0a00 WTFCrash
2   0x11358355c WebCore::RenderBlock::columnCount(WebCore::ColumnInfo*) const
3   0x1134bd9ff WebCore::Page::pageCount() const
4   0x10ec18f0d WebKit::WebPage::mainFrameDidLayout()
5   0x10eb672a3 WebKit::WebFrameLoaderClient::dispatchDidLayout()
6   0x112755b86 WebCore::FrameView::performPostLayoutTasks()
7   0x1127556fb WebCore::FrameView::layout(bool)
8   0x1123ddea9 WebCore::Document::implicitClose()
9   0x112719d3b WebCore::FrameLoader::checkCallImplicitClose()
10  0x1127199da WebCore::FrameLoader::checkCompleted()
11  0x112718278 WebCore::FrameLoader::finishedParsing()

[Dave Hyatt]

Created attachment 229809 [details]
Patch

Address the issues that caused the rollout.

[WebKit Commit Bot]

Attachment 229809 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderView.cpp:1266:  An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dean Jackson]

Comment on attachment 229809 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=229809&action=review

> Source/WebCore/rendering/RenderView.cpp:1268
> +    else if (multiColumnFlowThread())

It doesn't like the else here.

[Dave Hyatt]

Fix landed in r167597.