Bug 79645 – [CSSRegions]Implement NamedFlow::name attribute

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

[Mihnea Ovidenie]

http://dev.w3.org/csswg/css3-regions/#the-namedflow-interface

[Andrei Bucur]

Created attachment 143313 [details]
The implementation and the test

[Andreas Kling]

Comment on attachment 143313 [details]
The implementation and the test

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

Patch itself looks fine, but let's make the test a little more consistent with the rest of our layout tests.

Note that there's a helper script to create new tests in Tools/Scripts/make-new-script-test :)

> LayoutTests/ChangeLog:2
> [phone]  Andrei Bucur  <[e-mail]>
> +        [CSSRegions]Implement NamedFlow::name attribute

Missing newline between these rows.

> LayoutTests/fast/regions/webkit-named-flow-name-expected.txt:7
> +PASS
> +PASS
> +PASS

We should try to write tests that tell you a bit more about what's going right/wrong.

For example, this kind of construct in your test below:
assert(namedFlow.name == "flow-name", "The name should be 'flow-name' when there are no regions to flow into");

Could be replaced by our existing JS test helpers:
shouldBe("namedFlow.name", "'flow-name'");

As for the explanation of what's supposed to happen, that can just be a comment in the JS, or you can log them using the debug("foo bar baz"); helper.

[Andrei Bucur]

Created attachment 143558 [details]
Patch and test, second try

Patch with updated test case using Andreas suggestions. It looks better, I have to agree :).

[Andreas Kling]

Comment on attachment 143558 [details]
Patch and test, second try

Looks great. r=me

[WebKit Review Bot]

Comment on attachment 143558 [details]
Patch and test, second try

Clearing flags on attachment: 143558

Committed r118178: <http://trac.webkit.org/changeset/118178>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.