Bug 68665 - refactor RenderFlexibleBox to use flex-flow aware methods
Summary: refactor RenderFlexibleBox to use flex-flow aware methods
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: Ojan Vafai
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-22 17:04 PDT by Ojan Vafai
Modified: 2011-09-23 13:31 PDT (History)
2 users (show)

See Also:

Attachments
Patch (23.92 KB, patch)
2011-09-22 17:06 PDT, Ojan Vafai 		no flags Details | Formatted Diff | Diff
Patch (23.85 KB, patch)
2011-09-23 13:18 PDT, Ojan Vafai 		tony: review+
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 Ojan Vafai 2011-09-22 17:04:41 PDT 
refactor RenderFlexibleBox to use flex-flow aware methods
Comment 1 Ojan Vafai 2011-09-22 17:06:44 PDT 
Created attachment 108423 [details]
Patch
Comment 2 Tony Chang 2011-09-23 10:27:56 PDT 
Comment on attachment 108423 [details]
Patch

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

> Source/WebCore/rendering/RenderBlock.h:-213
> -    // FIXME: Supporting layout deltas.
> -    void setLogicalLocationForChild(RenderBox* child, const LayoutPoint&);

Can you add a line in the ChangeLog explaining why this is being removed (e.g., unused).

> Source/WebCore/rendering/RenderFlexibleBox.cpp:116
> +LayoutUnit RenderFlexibleBox::hasOrthogonalFlow(RenderBox* child) const

Shouldn't this return a bool?

> Source/WebCore/rendering/RenderFlexibleBox.cpp:124
> +LayoutUnit RenderFlexibleBox::isHorizontalFlow() const

Shouldn't this return a bool?

> Source/WebCore/rendering/RenderFlexibleBox.cpp:130
> +LayoutUnit RenderFlexibleBox::isLeftToRightFlow() const

Shouldn't this return a bool?

> Source/WebCore/rendering/RenderFlexibleBox.cpp:415
> +        // FIXME: I think we only want to do this for horizontal flows.

For vertical flows, we'll need something like this to compute the physical width.
Comment 3 Ojan Vafai 2011-09-23 13:18:14 PDT 
Created attachment 108523 [details]
Patch
Comment 4 Ojan Vafai 2011-09-23 13:31:21 PDT 
Committed r95856: <http://trac.webkit.org/changeset/95856>