RESOLVED FIXED 46639
Make computeLogicalHeight block-flow-aware.
https://bugs.webkit.org/show_bug.cgi?id=46639
Summary Make computeLogicalHeight block-flow-aware.
Dave Hyatt
Reported 2010-09-27 11:41:55 PDT
Make computeLogicalHeight block-flow-aware, and get layout limping enough that boxes have the correct widths/heights in vertical text mode.
Attachments
Patch (53.61 KB, patch)
2010-09-27 12:04 PDT, Dave Hyatt
sam: review+
Dave Hyatt
Comment 1 2010-09-27 12:04:38 PDT
WebKit Review Bot
Comment 2 2010-09-27 12:08:14 PDT
Attachment 68942 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/rendering/RenderBox.cpp:1834: An else should appear on the same line as the preceding } [whitespace/newline] [4] Total errors found: 1 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 3 2010-09-27 12:23:30 PDT
Comment on attachment 68942 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=68942&action=review > WebCore/rendering/RenderBox.cpp:1759 > + if (document()->printing()) > + visHeight = static_cast<int>(view()->pageHeight()); > + else if (style()->isVerticalBlockFlow()) > + visHeight = view()->viewHeight(); > + else > + visHeight = view()->viewWidth(); I think this might read clearer as an if-else: if (document()->printing() foo() else { if (style->verticalBlockFlow()) bar() else baz(); } That way, when paginated support comes, it is more clear where the branch needs to go. > LayoutTests/ChangeLog:39 > +2010-09-27 David Hyatt <hyatt@apple.com> > + > + Reviewed by NOBODY (OOPS!). > + > + Need a short description and bug URL (OOPS!) > + > + * fast/blockflow/auto-margins-across-boundaries-expected.txt: Added. > + * fast/blockflow/auto-margins-across-boundaries.html: Added. > + * fast/blockflow/block-formatting-context-expected.txt: Added. > + * fast/blockflow/block-formatting-context.html: Added. > + * fast/blockflow/percentage-padding.html: > + * fast/blockflow/relative-positioning-percentages-expected.txt: Added. > + * fast/blockflow/relative-positioning-percentages.html: Added. > + Double changelog entry! It think it probably makes sense to add some layout tests that don't use getClientRects (dumps the render tree the old fashion way) just incase that function is not returning the right results.
Dave Hyatt
Comment 4 2010-09-27 12:39:46 PDT
(In reply to comment #3) > (From update of attachment 68942 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=68942&action=review > > > WebCore/rendering/RenderBox.cpp:1759 > > + if (document()->printing()) > > + visHeight = static_cast<int>(view()->pageHeight()); > > + else if (style()->isVerticalBlockFlow()) > > + visHeight = view()->viewHeight(); > > + else > > + visHeight = view()->viewWidth(); > > I think this might read clearer as an if-else: > > if (document()->printing() > foo() > else { > if (style->verticalBlockFlow()) > bar() > else > baz(); > } > > That way, when paginated support comes, it is more clear where the branch needs to go. > Yeah, sounds good. > > LayoutTests/ChangeLog:39 > > +2010-09-27 David Hyatt <hyatt@apple.com> > > + > > + Reviewed by NOBODY (OOPS!). > > + > > + Need a short description and bug URL (OOPS!) > > + > > + * fast/blockflow/auto-margins-across-boundaries-expected.txt: Added. > > + * fast/blockflow/auto-margins-across-boundaries.html: Added. > > + * fast/blockflow/block-formatting-context-expected.txt: Added. > > + * fast/blockflow/block-formatting-context.html: Added. > > + * fast/blockflow/percentage-padding.html: > > + * fast/blockflow/relative-positioning-percentages-expected.txt: Added. > > + * fast/blockflow/relative-positioning-percentages.html: Added. > > + > > Double changelog entry! > > It think it probably makes sense to add some layout tests that don't use getClientRects (dumps the render tree the old fashion way) just incase that function is not returning the right results. The reason I am not doing this yet is just because the x/y positions aren't right yet. This way I have tests that pass as long as width/height are good and that won't break as I work on x/y. As more of layout comes online, I'll fall back to traditional render tree dumps yes.
Dave Hyatt
Comment 5 2010-09-27 12:43:09 PDT
Fixed in r68417.
WebKit Review Bot
Comment 6 2010-09-27 14:05:15 PDT
Dave Hyatt
Comment 7 2010-09-27 14:06:08 PDT
It did. See https://bugs.webkit.org/show_bug.cgi?id=46649 I will have a fix shortly.
Note You need to log in before you can comment on or make changes to this bug.