WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
46519
Convert the implementation of computeLogicalWidth to work with block-flow
https://bugs.webkit.org/show_bug.cgi?id=46519
Summary
Convert the implementation of computeLogicalWidth to work with block-flow
Dave Hyatt
Reported
2010-09-24 14:49:11 PDT
Convert the implementation of computeLogicalWidth to work with block-flow. This bug addresses patching the RenderBox computeLogicalWidth function to be block-flow-aware. It does not cover positioned elements, marquees, flexible boxes, table, etc... just basic elements.
Attachments
Patch
(149.13 KB, patch)
2010-09-24 15:10 PDT
,
Dave Hyatt
no flags
Details
Formatted Diff
Diff
Clean up the ChangeLog typos.
(149.12 KB, patch)
2010-09-24 15:14 PDT
,
Dave Hyatt
sam
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Dave Hyatt
Comment 1
2010-09-24 15:10:03 PDT
Created
attachment 68765
[details]
Patch
Dave Hyatt
Comment 2
2010-09-24 15:14:28 PDT
Created
attachment 68766
[details]
Clean up the ChangeLog typos.
Sam Weinig
Comment 3
2010-09-26 18:00:53 PDT
Comment on
attachment 68766
[details]
Clean up the ChangeLog typos. View in context:
https://bugs.webkit.org/attachment.cgi?id=68766&action=review
> WebCore/rendering/RenderBox.cpp:1492 > + // Fieldsets do this.
This comment could be improved.
> WebCore/rendering/RenderBox.cpp:1613 > + Length marginStartLength = style()->marginStartUsing(containingBlock->style()); > + Length marginEndLength = style()->marginEndUsing(containingBlock->style()); > + const RenderStyle* containingBlockStyle = containingBlock->style(); > + > + // Case One: The object is being centered in the containing block's available logical width. > + if ((marginStartLength.isAuto() && marginEndLength.isAuto() && childWidth < containerWidth) > + || (!marginStartLength.isAuto() && !marginEndLength.isAuto() && containingBlock->style()->textAlign() == WEBKIT_CENTER)) { > + setMarginStartUsing(containingBlockStyle, max(0, (containerWidth - childWidth) / 2)); > + setMarginEndUsing(containingBlockStyle, containerWidth - childWidth - marginStartUsing(containingBlockStyle)); > + return; > + } > + > + // Case Two: The object is being pushed to the start of the containing block's available logical width. > + if (marginEndLength.isAuto() && childWidth < containerWidth) { > + setMarginStartUsing(containingBlockStyle, marginStartLength.calcValue(containerWidth)); > + setMarginEndUsing(containingBlockStyle, containerWidth - childWidth - marginStartUsing(containingBlockStyle)); > + return; > + } > + > + // Case Three: The object is being pushed to the end of the containing block's available logical width. > + bool pushToEndFromTextAlign = !marginEndLength.isAuto() && ((containingBlockStyle->direction() == RTL && containingBlockStyle->textAlign() == WEBKIT_LEFT) > + || (containingBlockStyle->direction() == LTR && containingBlockStyle->textAlign() == WEBKIT_RIGHT)); > + if ((marginStartLength.isAuto() && childWidth < containerWidth) || pushToEndFromTextAlign) { > + setMarginEndUsing(containingBlockStyle, marginEndLength.calcValue(containerWidth)); > + setMarginStartUsing(containingBlockStyle, containerWidth - childWidth - marginEndUsing(containingBlockStyle)); > + return; > + } > + > + // Case Four: No auto margins. Just compute normally. > + // This makes auto margins 0 if we failed a logicalWidth() < containerWidth test above (css2.1, 10.3.3). > + setMarginStartUsing(containingBlockStyle, marginStartLength.calcMinValue(containerWidth)); > + setMarginEndUsing(containingBlockStyle, marginEndLength.calcMinValue(containerWidth));
I am not sure how hot these functions are, but we could avoid some branches by adding marginStartAndEndUsing()/setMarginStartAndEndUsing() style functions to get/set both at once. You might also consider having the setMargin functions return the length they set, since you often use it on the next line.
> WebCore/rendering/RenderBox.h:198 > + // of the containing block..
Typo. Double period.
Dave Hyatt
Comment 4
2010-09-26 19:09:55 PDT
Fixed in
r68362
.
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