Bug 96129

Summary:

Make RenderBox::computeLogicalWidthInRegion const

Product:

WebKit

Reporter:

Tony Chang <tony>

Component:

Layout and Rendering

Assignee:

Tony Chang <tony>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

eric, hyatt, ojan, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

94982

Attachments:

Description	Flags
Patch	none
Patch for landing	none

Tony Chang

Reported 2012-09-07 10:13:29 PDT

This allows us to get rid of the hack in RenderBox::renderBoxRegionInfo.

Attachments
Patch (16.78 KB, patch) 2012-09-07 10:14 PDT, Tony Chang	no flags	Details Formatted Diff Diff
Patch for landing (16.76 KB, patch) 2012-09-07 10:50 PDT, Tony Chang	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Tony Chang

Comment 1 2012-09-07 10:14:47 PDT

Created attachment 162799 [details] Patch

Tony Chang

Comment 2 2012-09-07 10:18:47 PDT

Comment on attachment 162799 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162799&action=review > Source/WebCore/rendering/RenderBox.cpp:1706 > + bool hasInvertedDirection = cb->style()->isLeftToRightDirection() != style()->isLeftToRightDirection(); > + computeInlineDirectionMargins(cb, containerLogicalWidthForAutoMargins, computedValues.m_extent, > + hasInvertedDirection ? computedValues.m_margins.m_end : computedValues.m_margins.m_start, > + hasInvertedDirection ? computedValues.m_margins.m_start : computedValues.m_margins.m_end); This variable was named incorrectly. I made the name correct by flipping the expression. > Source/WebCore/rendering/RenderBox.cpp:1715 > + bool hasInvertedDirection = cb->style()->isLeftToRightDirection() != style()->isLeftToRightDirection(); > + if (hasInvertedDirection) > + computedValues.m_margins.m_start = containerLogicalWidth - computedValues.m_extent - cb->marginStartForChild(this); > + else > + computedValues.m_margins.m_end = containerLogicalWidth - computedValues.m_extent - cb->marginStartForChild(this); I could make a helper function for this on MarginValues if you think it helps.

Ojan Vafai

Comment 3 2012-09-07 10:29:04 PDT

Comment on attachment 162799 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162799&action=review >> Source/WebCore/rendering/RenderBox.cpp:1715 >> + computedValues.m_margins.m_end = containerLogicalWidth - computedValues.m_extent - cb->marginStartForChild(this); > > I could make a helper function for this on MarginValues if you think it helps. Can you just create a local variable before the if/else? LayoutUnit newMargin = containerLogicalWidth - computedValues.m_extent - cb->marginStartForChild(this);

Tony Chang

Comment 4 2012-09-07 10:50:51 PDT

Created attachment 162815 [details] Patch for landing

Tony Chang

Comment 5 2012-09-07 10:51:13 PDT

Comment on attachment 162815 [details] Patch for landing Want to let the ews bots chew on this first.

WebKit Review Bot

Comment 6 2012-09-07 13:50:18 PDT

Comment on attachment 162815 [details] Patch for landing Clearing flags on attachment: 162815 Committed r127914: <http://trac.webkit.org/changeset/127914>

WebKit Review Bot

Comment 7 2012-09-07 13:50:21 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.