Bug 68665

Summary:

refactor RenderFlexibleBox to use flex-flow aware methods

Product:

WebKit

Reporter:

Ojan Vafai <ojan>

Component:

New Bugs

Assignee:

Ojan Vafai <ojan>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

hyatt, tony

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	tony: review+

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>