Bug 63133 – [CSSRegions]Parse content: -webkit-from-flow

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Mihnea Ovidenie]

Add support for css regions content: from-flow: http://www.w3.org/TR/css3-regions/#the-content-property.

[Mihnea Ovidenie]

Created attachment 99084 [details]
Patch

[Mihnea Ovidenie]

Created attachment 99470 [details]
Patch 2

Moved tests into LayoutTests/fast/regions

[Dave Hyatt]

Comment on attachment 99470 [details]
Patch 2

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

Couple of nits.

> Source/WebCore/css/CSSParser.h:160
> +        PassRefPtr<CSSValue> parseFromFlowContent(CSSParserValueList* args);

You can omit the word "args" here. Typically if the type of the parameter makes it obvious what you've got, we just omit the name in headers.

> Source/WebCore/css/CSSStyleSelector.cpp:4165
> +

Useless whitespace change. Get rid of this. Thanks.

> Source/WebCore/rendering/style/RenderStyle.h:1099
> +    void setRegionThread(const AtomicString& n) { SET_VAR(rareNonInheritedData, m_regionThread, n); }

Can omit "n" here.

[Mihnea Ovidenie]

Created attachment 99958 [details]
Patch 3

Thanks for the review. I have reworked the patch based on your suggestions and taking into account the previous CSSRegions patch that got into trunk.

[Dave Hyatt]

Comment on attachment 99958 [details]
Patch 3

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

Looks good but I forgot you need the -webkit- prefix for any new values you add to already-standardized properties.

> Source/WebCore/css/CSSParser.cpp:2661
> +            } else if (equalIgnoringCase(val->function->name, "from-flow(")) {

Seems like this needs to be "-webkit-from-flow", since we're putting it into an existing property, and this value is still non-standard.

> Source/WebCore/css/CSSPrimitiveValue.cpp:695
> +            text = "from-flow(" + quoteCSSStringIfNeeded(m_value.string) + ")";

Same here... -webkit-from-flow and not just from-flow.

[Mihnea Ovidenie]

Created attachment 100083 [details]
Patch 4

Reworking the patch:
1. Changed from content: from-flow to content: -webkit-from-flow. 
2. Reworked tests to take into account -webkit-from-flow
3. Renamed test files

[Dave Hyatt]

Comment on attachment 100083 [details]
Patch 4

r=me

[WebKit Review Bot]

Comment on attachment 100083 [details]
Patch 4

Clearing flags on attachment: 100083

Committed r90642: <http://trac.webkit.org/changeset/90642>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.