Bug 74008 - add css parsing for flex-flow: wrap and wrap-reverse
Summary: add css parsing for flex-flow: wrap and wrap-reverse
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tony Chang
URL:
Keywords:
Depends on:
Blocks: 70781
  Show dependency treegraph
 
Reported: 2011-12-07 10:20 PST by Tony Chang
Modified: 2011-12-09 15:50 PST (History)
5 users (show)

See Also:


Attachments
Patch (19.46 KB, patch)
2011-12-07 10:26 PST, Tony Chang
no flags Details | Formatted Diff | Diff
Patch (21.79 KB, patch)
2011-12-07 13:42 PST, Tony Chang
no flags Details | Formatted Diff | Diff
Patch for landing (21.79 KB, patch)
2011-12-09 15:44 PST, 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 Tony Chang 2011-12-07 10:20:36 PST
add css parsing for flex-flow: wrap and wrap-reverse
Comment 1 Tony Chang 2011-12-07 10:26:05 PST
Created attachment 118229 [details]
Patch
Comment 2 Ojan Vafai 2011-12-07 10:54:03 PST
Comment on attachment 118229 [details]
Patch

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

> Source/WebCore/css/CSSPrimitiveValueMappings.h:1246
> +        m_value.ident = CSSValueNone;

Should this be CSSValueNoWrap?
Comment 3 WebKit Review Bot 2011-12-07 12:45:22 PST
Comment on attachment 118229 [details]
Patch

Attachment 118229 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10747260

New failing tests:
svg/css/getComputedStyle-basic.xhtml
Comment 4 Tony Chang 2011-12-07 13:42:33 PST
Created attachment 118267 [details]
Patch
Comment 5 Tony Chang 2011-12-07 13:43:13 PST
New version to hand the new syntax: [ row | row-reverse | column | column-reverse ] || [ nowrap | wrap | wrap-reverse ]?
Comment 6 Darin Adler 2011-12-07 14:33:46 PST
Comment on attachment 118267 [details]
Patch

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

> Source/WebCore/css/CSSParser.cpp:1639
> +            if (value->id == CSSValueNowrap || value->id == CSSValueWrap || value->id == CSSValueWrapReverse) {
> +                list->append(cssValuePool()->createIdentifierValue(value->id));
> +                addProperty(propId, list, important);
> +                return true;
> +            }
> +            return false;

We normally use early return for the failure case. Reversing this if would make it easier to read the normal flow.
Comment 7 WebKit Review Bot 2011-12-07 18:24:44 PST
Comment on attachment 118267 [details]
Patch

Attachment 118267 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10780029

New failing tests:
svg/css/getComputedStyle-basic.xhtml
fast/css/getComputedStyle/computed-style.html
fast/css/getComputedStyle/computed-style-without-renderer.html
Comment 8 Tony Chang 2011-12-09 15:44:55 PST
Created attachment 118660 [details]
Patch for landing
Comment 9 Ojan Vafai 2011-12-09 15:50:25 PST
Comment on attachment 118660 [details]
Patch for landing

Clearing flags on attachment: 118660

Committed r102486: <http://trac.webkit.org/changeset/102486>
Comment 10 Ojan Vafai 2011-12-09 15:50:30 PST
All reviewed patches have been landed.  Closing bug.