WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Dave Hyatt
Comment 1
2010-09-27 12:04:38 PDT
Created
attachment 68942
[details]
Patch
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
http://trac.webkit.org/changeset/68417
might have broken Leopard Intel Debug (Tests) The following changes are on the blame list:
http://trac.webkit.org/changeset/68416
http://trac.webkit.org/changeset/68417
http://trac.webkit.org/changeset/68415
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.
Top of Page
Format For Printing
XML
Clone This Bug