Bug 58557

Summary:

Extract layoutRepacedElements from layoutInlineChildren

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

Layout and Rendering

Assignee:

Nobody <webkit-unassigned>

Status:

NEW

Severity:

Enhancement

CC:

cmarcelo, dglazkov, eric, hyatt

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

57779

Attachments:

Description	Flags
cleanup	eric: review-

Ryosuke Niwa

Reported 2011-04-14 11:19:20 PDT

This is a cleanup.

Attachments
cleanup (6.70 KB, patch) 2011-04-14 11:21 PDT, Ryosuke Niwa	eric: review-	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Ryosuke Niwa

Comment 1 2011-04-14 11:21:01 PDT

Created attachment 89612 [details] cleanup

Eric Seidel (no email)

Comment 2 2011-04-14 11:42:54 PDT

Comment on attachment 89612 [details] cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=89612&action=review > Source/WebCore/rendering/RenderBlockLineLayout.cpp:716 > +inline void RenderBlock::layoutRepacedElements(bool relayoutChildren, bool fullLayout, Vector<RenderBlock::FloatWithRect>& floats, bool& hasInlineChild) I thought about doing just this. HOweer, this does more than just layout replaced elments. It's sorta a first-pass layout. I wasn't really sure what to call it when I wrote a similar patch.

Daniel Bates

Comment 3 2011-04-14 11:48:21 PDT

Comment on attachment 89612 [details] cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=89612&action=review > Source/WebCore/ChangeLog:8 > + Extracted a block of code in layoutInlineChildren as layoutRepacedElements. layoutRepacedElements => layoutReplacedElements (you used layoutRepacedElements throughout this patch) And, as Eric Seidel remarked, this function seems to more than just handle replace elements. > Source/WebCore/rendering/RenderBlockLineLayout.cpp:740 > + } else if (object->isText() || (object->isRenderInline() && !endOfInline)) { Nit: The parentheses around the second disjunct are unnecessary since && (logical and) has higher precedence than || (logical or).

Ryosuke Niwa

Comment 4 2011-04-14 12:03:58 PDT

Comment on attachment 89612 [details] cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=89612&action=review >> Source/WebCore/ChangeLog:8 >> + Extracted a block of code in layoutInlineChildren as layoutRepacedElements. > > layoutRepacedElements => layoutReplacedElements > > (you used layoutRepacedElements throughout this patch) > > And, as Eric Seidel remarked, this function seems to more than just handle replace elements. Oops. I'll rename the function. Any suggestion? >> Source/WebCore/rendering/RenderBlockLineLayout.cpp:740 >> + } else if (object->isText() || (object->isRenderInline() && !endOfInline)) { > > Nit: The parentheses around the second disjunct are unnecessary since && (logical and) has higher precedence than || (logical or). There's at least one port where not having this parenthesis causes a build failure because new versions of gcc gives you a warning on this.

Eric Seidel (no email)

Comment 5 2011-04-26 16:18:36 PDT

Comment on attachment 89612 [details] cleanup We need a better name. We'll work on this mor in our hackathon on thurs.

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