Bug 101294

Created attachment 172473 [details] Patch

If absolute div before all flex item in a container,and the set flex container to "-webkit-flex-flow: row wrap" the flex item after absolute div will move to a new line.

We may come across this problem in RenderFlexibleBox::computeNextFlexLine.when it comes to the move-to-next-line issue, 2 condition must be taken into consideration 1) whether the width has gone beyond lineBreakLength , 2) whether the rderedChildren is empty. If the width has gone beyond ineBreakLength and absloute layout is ongoing in the container, wrong line-moving might occur. The way to solve the problem is to jutify whether orderedChildren could collect at least one flex item, if yes, move to next line

Created attachment 172511 [details] Prev patch lost a LayoutTesst Changelog.

Created attachment 172525 [details] Add more Changelog

Created attachment 172560 [details] Finally patch

Created attachment 172566 [details] Really finally patch

Comment on attachment 172566 [details] Really finally patch 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.

Created attachment 172802 [details] Patch

Comment on attachment 172802 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=172802&action=review > Source/WebCore/ChangeLog:8 > + Additional information of the change such as approach, rationale. Please add per-function descriptions below (OOPS!). You need to put a description of the change here. > Source/WebCore/ChangeLog:10 > + No new tests (OOPS!). You can delete this line. > LayoutTests/ChangeLog:8 > + Additional information of the change such as approach, rationale. Please add per-function descriptions below (OOPS!). Need a short description here.

Created attachment 172928 [details] Patch

Comment on attachment 172928 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=172928&action=review This patch doesn't seem to apply to tip of tree (see all the bot failures). > Source/WebCore/ChangeLog:10 > + If there is an absolutely positioned flex item before other items in a container and the container is set to "-webkit-flex-flow: row wrap", the flex item after the absolutely positioned will move to a new line. > + This issue has to do with RenderFlexibleBox::computeNextFlexLine. When determing line breaks, the algorithm sees if 1) the total width exceeds lineBreakLength and 2) whether orderedChildren is non-empty. But then, if the total width exceeds lineBreakLength and there's only absolutely positioned elemments in orderedChildren then the conditions are met and the algorithm mistakenly breaks the line. > + The solution is to see if orderedChildren collects at least a flex item. If it does, break the line. Please use only spaces and not tabs.

Created attachment 172931 [details] Patch

Comment on attachment 172931 [details] Patch Patch looks good. Thanks. Just need to figure out why it's not applying. Looks like there's a conflict in RenderFlexibleBox.cpp. e.g. see http://queues.webkit.org/results/14757641.

Created attachment 172975 [details] Patch

Created attachment 172986 [details] Patch

Comment on attachment 172986 [details] Patch Yay! Thanks! I assume you want me to commit this as well. I'm adding cq+ (puts in in the commit queue). For future reference, you request cq by setting cq?. That lets the reviewer know you would like a cq+ once the patch is r+'ed.

Comment on attachment 172986 [details] Patch Clearing flags on attachment: 172986 Committed r133906: <http://trac.webkit.org/changeset/133906>

All reviewed patches have been landed. Closing bug.

Comment on attachment 172975 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=172975&action=review > LayoutTests/css3/flexbox/flex-algorithm.html:201 > +<div class="flexbox" style="height: 60px; -webkit-flex-flow: row wrap; position: relative;"> Nit: Instead of using -webkit-flex-flow, you should use the row and wrap class from flexbox.css so this code will run in Firefox.