Bug 95907 - Make computePositionedLogicalWidth and computePositionedLogicalWidthReplaced const
Summary: Make computePositionedLogicalWidth and computePositionedLogicalWidthReplaced ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tony Chang
URL:
Keywords:
Depends on:
Blocks: 94982
  Show dependency tree / graph
 
Reported: 2012-09-05 16:29 PDT by Tony Chang
Modified: 2012-09-06 19:42 PDT (History)
3 users (show)

See Also:

Attachments
Patch (24.82 KB, patch)
2012-09-05 16:34 PDT, Tony Chang 		no flags Details | Formatted Diff | Diff
Patch (24.01 KB, patch)
2012-09-06 11:46 PDT, Tony Chang 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Chang 2012-09-05 16:29:36 PDT 
Make computePositionedLogicalWidth and computePositionedLogicalWidthReplaced const
Comment 1 Tony Chang 2012-09-05 16:34:33 PDT 
Created attachment 162367 [details]
Patch
Comment 2 Tony Chang 2012-09-05 16:39:29 PDT 
This parallels the changes to computePositionedLogicalHeight/computePositionedLogicalHeightReplaced in http://trac.webkit.org/changeset/126802 .
Comment 3 Ojan Vafai 2012-09-05 18:32:23 PDT 
Comment on attachment 162367 [details]
Patch

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

> Source/WebCore/rendering/RenderBox.cpp:2676
> +                                           stretchValues);
> +        computedValues.m_extent = stretchValues.m_extent;
> +        computedValues.m_position = stretchValues.m_position;
> +        computedValues.m_margins.m_start = stretchValues.m_margins.m_start;
> +        computedValues.m_margins.m_end = stretchValues.m_margins.m_end;

Why can't you just pass computedValues in instead of stretchValues?

> Source/WebCore/rendering/RenderBox.cpp:3184
> +    LayoutUnit& marginLogicalLeftAlias = style()->isLeftToRightDirection() ? computedValues.m_margins.m_start : computedValues.m_margins.m_end;
> +    LayoutUnit& marginLogicalRightAlias = style()->isLeftToRightDirection() ? computedValues.m_margins.m_end : computedValues.m_margins.m_start;

Here and elsewhere, these don't look equivalent to me. 

FractionalLayoutUnit& FractionalLayoutBoxExtent::mutableLogicalLeft(WritingMode writingMode)
{
    return isHorizontalWritingMode(writingMode) ? m_left : m_top;
}
Comment 4 Tony Chang 2012-09-06 11:46:33 PDT 
Created attachment 162546 [details]
Patch
Comment 5 Tony Chang 2012-09-06 11:48:04 PDT 
Comment on attachment 162546 [details]
Patch

Removed stretchValues.

The new code is the same because we want to ignore ltr/rtl, but start/end take that into consideration.  In other words, it's undoing the flipping caused by ltr/rtl.
Comment 6 WebKit Review Bot 2012-09-06 19:42:54 PDT 
Comment on attachment 162546 [details]
Patch

Clearing flags on attachment: 162546

Committed r127812: <http://trac.webkit.org/changeset/127812>
Comment 7 WebKit Review Bot 2012-09-06 19:42:57 PDT 
All reviewed patches have been landed.  Closing bug.