Summary: | flexbox flexing implementation should match the spec | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ojan Vafai <ojan> | ||||||||
Component: | Layout and Rendering | Assignee: | Tony Chang <tony> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | hyatt, tony, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 62048 | ||||||||||
Attachments: |
|
Description
Ojan Vafai
2011-10-24 19:35:21 PDT
The spec hasn't been updated, but the current thinking is here: http://lists.w3.org/Archives/Public/www-style/2011Oct/0533.html With a JS implementation here: http://www.xanthir.com/etc/flexplay/test.html Created attachment 133106 [details]
Patch
Created attachment 133309 [details]
Patch
(In reply to comment #3) > Created an attachment (id=133309) [details] > Patch Rebase to ToT and drop m_ from struct member variables. Comment on attachment 133309 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133309&action=review > Source/WebCore/rendering/RenderFlexibleBox.cpp:787 > + Length childMaxWidth = isHorizontalFlow() ? child->style()->maxWidth() : child->style()->maxHeight(); > + Length childMinWidth = isHorizontalFlow() ? child->style()->minWidth() : child->style()->minHeight(); If you move these into adjustFlexSizeForMinAndMax, then you could use adjustFlexSizeForMinAndMax to replace lines 716-722 of computeNextFlexLine as well. > Source/WebCore/rendering/RenderFlexibleBox.cpp:798 > + childSizes.append(childSize); Nit: You could move this to replace line 778 and 790. Then you would only need the childSize variable in the else clause. Seems a bit cleaner to me. > Source/WebCore/rendering/RenderFlexibleBox.cpp:805 > + if (!totalViolation) > + return true; > + > + freezeViolations(totalViolation < 0 ? maxViolations : minViolations, availableFreeSpace, totalPositiveFlexibility, totalNegativeFlexibility, inflexibleItems); > + return false; You could write this as: if (totalViolation) freezeViolations(...); return !totalViolation Not sure that's better though. It's fewer lines of code. Created attachment 133333 [details]
Patch
Comment on attachment 133333 [details]
Patch
I made all the changes (it looks much better). I'm going to let the bots chew through this before landing.
Comment on attachment 133333 [details] Patch Clearing flags on attachment: 133333 Committed r111767: <http://trac.webkit.org/changeset/111767> All reviewed patches have been landed. Closing bug. |