[Webkit-unassigned] [Bug 101294] While absolute positioning is put before the first flexitem, flexitems will move to a new line.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Nov 6 09:31:06 PST 2012


https://bugs.webkit.org/show_bug.cgi?id=101294





--- Comment #8 from Ojan Vafai <ojan at chromium.org>  2012-11-06 09:32:38 PST ---
(From update of attachment 172566)
View in context: https://bugs.webkit.org/attachment.cgi?id=172566&action=review

Thanks for the fix. The change looks good. Just a bunch of style nits.

You're ChangeLog description does not match the standard format. I recommend deleting them and uploading this patch using "webkit-patch upload". Then it will create the ChangeLog entries for you and you just need to fill in the detailed description section. Also, that will properly mark the patch as ready for review by setting r?.

> LayoutTests/css3/flexbox/flex-algorithm.html:202
> +  <div style="position: absolute;"></div>

May as well check the x/y positions on this element as well.

> LayoutTests/css3/flexbox/flex-algorithm.html:203
> +  <div data-offset-y="0" style="width: 700px;"></div>

May as well add data-offset-x=0 as well for good measure.

> Source/WebCore/rendering/RenderFlexibleBox.cpp:898
> +    bool hasOneItemCollected = false;

I'd prefer a name like...lineHasInFlowItem. Also, a line break after this line would break up the code to make it a bit more readable.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list