Bug 82128 - Implement new flex property and deprecate flex function
Summary: Implement new flex property and deprecate flex function
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Tony Chang
URL:
Keywords:
Depends on: 82927
Blocks: 62048
  Show dependency treegraph
 
Reported: 2012-03-24 10:09 PDT by niclas
Modified: 2012-04-03 15:34 PDT (History)
4 users (show)

See Also:


Attachments
Patch (174.87 KB, patch)
2012-04-03 13:43 PDT, Tony Chang
no flags Details | Formatted Diff | Diff
Patch for landing (177.82 KB, patch)
2012-04-03 13:59 PDT, Tony Chang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description niclas 2012-03-24 10:09:46 PDT
The new flexbox spec was released on 22nd of March.

The flex function has been removed in favour of the flex property
http://www.w3.org/TR/css3-flexbox/#flexibility
Comment 1 Tony Chang 2012-04-03 13:43:29 PDT
Created attachment 135412 [details]
Patch
Comment 2 Tony Chang 2012-04-03 13:45:25 PDT
The code change was surprisingly small.  I left the old css parsing in this change.  I'll remove it separately.
Comment 3 Ojan Vafai 2012-04-03 13:52:16 PDT
Comment on attachment 135412 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=135412&action=review

> Source/WebCore/rendering/RenderFlexibleBox.cpp:348
> +Length RenderFlexibleBox::preferredFlexLengthForChild(RenderBox* child) const

Not a huge fan of this name. I feel like it could easily be confused with the length after we've done the flex algorithm. How about mainAxisPreferredLengthForChild?

> Source/WebCore/rendering/RenderFlexibleBox.cpp:650
>  float RenderFlexibleBox::positiveFlexForChild(RenderBox* child) const
>  {
> -    return isHorizontalFlow() ? child->style()->flexboxWidthPositiveFlex() : child->style()->flexboxHeightPositiveFlex();
> +    return child->style()->positiveFlex();
>  }
>  
>  float RenderFlexibleBox::negativeFlexForChild(RenderBox* child) const
>  {
> -    return isHorizontalFlow() ? child->style()->flexboxWidthNegativeFlex() : child->style()->flexboxHeightNegativeFlex();
> +    return child->style()->negativeFlex();
>  }

Don't necessarily need to do it in this patch, but these helper functions don't seem terribly useful to me.
Comment 4 Tony Chang 2012-04-03 13:59:30 PDT
Created attachment 135419 [details]
Patch for landing
Comment 5 Tony Chang 2012-04-03 14:00:25 PDT
(In reply to comment #3)
> (From update of attachment 135412 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=135412&action=review
> 
> > Source/WebCore/rendering/RenderFlexibleBox.cpp:348
> > +Length RenderFlexibleBox::preferredFlexLengthForChild(RenderBox* child) const
> 
> Not a huge fan of this name. I feel like it could easily be confused with the length after we've done the flex algorithm. How about mainAxisPreferredLengthForChild?

Renamed to preferredLengthForChild.

> > Source/WebCore/rendering/RenderFlexibleBox.cpp:650
> >  float RenderFlexibleBox::positiveFlexForChild(RenderBox* child) const
> >  {
> > -    return isHorizontalFlow() ? child->style()->flexboxWidthPositiveFlex() : child->style()->flexboxHeightPositiveFlex();
> > +    return child->style()->positiveFlex();
> >  }
> >  
> >  float RenderFlexibleBox::negativeFlexForChild(RenderBox* child) const
> >  {
> > -    return isHorizontalFlow() ? child->style()->flexboxWidthNegativeFlex() : child->style()->flexboxHeightNegativeFlex();
> > +    return child->style()->negativeFlex();
> >  }
> 
> Don't necessarily need to do it in this patch, but these helper functions don't seem terribly useful to me.

Inlined.
Comment 6 WebKit Review Bot 2012-04-03 15:34:25 PDT
Comment on attachment 135419 [details]
Patch for landing

Clearing flags on attachment: 135419

Committed r113097: <http://trac.webkit.org/changeset/113097>
Comment 7 WebKit Review Bot 2012-04-03 15:34:31 PDT
All reviewed patches have been landed.  Closing bug.