New tests for CSS2 corresponding to the changes introduced by CSS 2.2

[bert-github]

Some 30-odd new tests for the items in http://www.w3.org/TR/CSS22/changes.html
The tests are added in the css21 directory on the assumption that that directory is for all CSS level 2 tests, despite the name.

---

This change is

[syncbot]

Automatic validation checks of commit e7be31f discovered the following problems:

In css21/margin-padding-clear/margin-collapse-min-height-001.xht:

• Not linked to a tracked specification anchor.

In css21/syntax/signed-numbers-001.xht:

• Not linked to a tracked specification anchor.

In css21/syntax/signed-numbers-001-ref.xht:

• Reference must not have specification links.
• Reference must not have assertions.

In css21/cascade/inherit-computed-001.html:

• Not linked to a tracked specification anchor.

In css21/visudet/height-percentage-004-ref.xht:

• Reference must not have specification links.
• Reference must not have assertions.

In css21/syntax/characters-0080-009F-001-ref.xht:

• Reference must not have specification links.
• Reference must not have assertions.

In css21/syntax/escaped-url-001-ref.xht:

• Reference must not have specification links.
• Reference must not have assertions.

In css21/fonts/font-045.xht:

• Line 10: Unknown anchor changes.html#q142 in specification link to css21.

In css21/cascade/inherit-computed-002.html:

• Not linked to a tracked specification anchor.

[gsnedders]

w3c-test:mirror

[gsnedders]

Using something that isn't font-size would seem better here, given if the span rule isn't applied or the font-size: inherit ignored this test will still pass. Seems much better to use a property that isn't inherited by default (border or display, maybe?).

[gsnedders]

Missing >

[gsnedders]

Missing >

[gsnedders]

This being position: relative doesn't have an effect, does it?

[gsnedders]

I don't understand that statement. There's more… what, above? "There must be more whitespace above this sentence than there is between this and the horizontal rule"?

https://www.w3.org/Bugs/Public/show_bug.cgi?id=16036 includes what seems like a better, easier to understand test of a more normal form (i.e., green boxes, no red). It's much easier to distinguish no red than differing amounts of whitespace!

[gsnedders]

Given this is XHTML and not HTML, can we not just use character references for them?

[gsnedders]

Please also add a character-encoding-041.css.headers file.

[gsnedders]

lightblue isn't a colour in 2.2, suggest aqua instead.

[gsnedders]

This doesn't exist in this commit (though it's added later in c90c4c42cc8fef3587897ba32eb90d35ef0d82fa) if you care about having a clean history (it certainly makes reviewing easier!)

[gsnedders]

In case GitHub eats my earlier comment: lightblue isn't a colour keyword in 2.2.

[syncbot]

Automatic validation checks of commit e98d808 discovered the following problems:

In css21/margin-padding-clear/margin-collapse-min-height-001.xht:

• Not linked to a tracked specification anchor.

In css21/syntax/signed-numbers-001.xht:

• Not linked to a tracked specification anchor.

In css21/cascade/inherit-computed-001.html:

• Not linked to a tracked specification anchor.

In css21/syntax/signed-numbers-002.xht:

• Not linked to a tracked specification anchor.

In css21/margin-padding-clear/margin-collapse-min-height-002.xht:

• Not linked to a tracked specification anchor.

In css21/margin-padding-clear/margin-collapse-min-height-002-ref.xht:

• Reference must not have specification links.
• Reference must not have assertions.

In css21/fonts/font-045.xht:

• Line 10: Unknown anchor changes.html#q142 in specification link to css21.

In css21/cascade/inherit-computed-002.html:

• Not linked to a tracked specification anchor.

[syncbot]

Automatic validation checks of commit d031e72 discovered the following problems:

In css21/margin-padding-clear/margin-collapse-min-height-001.xht:

• Not linked to a tracked specification anchor.

In css21/syntax/signed-numbers-001.xht:

• Not linked to a tracked specification anchor.

In css21/cascade/inherit-computed-001.html:

• Not linked to a tracked specification anchor.

In css21/syntax/signed-numbers-002.xht:

• Not linked to a tracked specification anchor.

In css21/margin-padding-clear/margin-collapse-min-height-002.xht:

• Not linked to a tracked specification anchor.

In css21/margin-padding-clear/margin-collapse-min-height-002-ref.xht:

• Reference must not have specification links.
• Reference must not have assertions.

In css21/fonts/font-045.xht:

• Line 10: Unknown anchor changes.html#q142 in specification link to css21.

In css21/cascade/inherit-computed-002.html:

• Not linked to a tracked specification anchor.

[syncbot]

Automatic validation checks of commit 54e287a discovered the following problems:

In css21/margin-padding-clear/margin-collapse-min-height-001.xht:

• Not linked to a tracked specification anchor.

In css21/syntax/signed-numbers-001.xht:

• Not linked to a tracked specification anchor.

In css21/cascade/inherit-computed-001.html:

• Not linked to a tracked specification anchor.

In css21/syntax/signed-numbers-002.xht:

• Not linked to a tracked specification anchor.

In css21/margin-padding-clear/margin-collapse-min-height-002.xht:

• Not linked to a tracked specification anchor.

In css21/margin-padding-clear/margin-collapse-min-height-002-ref.xht:

• Reference must not have specification links.
• Reference must not have assertions.

In css21/fonts/font-045.xht:

• Line 10: Unknown anchor changes.html#q142 in specification link to css21.

In css21/cascade/inherit-computed-002.html:

• Not linked to a tracked specification anchor.

[bert-github]

Thanks for your review! Some comments and questions inline:

On 30 Oct 2016, at 22:47, Geoffrey Sneddon notifications@github.com wrote:

@gsnedders requested changes on this pull request.

In css21/cascade/inherit-computed-001.html:

@@ -0,0 +1,14 @@
+
+
+<title>CSS cascading: Inherit the computed value</title>
+
+
+
+<link rel="author" title="Bert Bos" href="mailto:bert@w3.org"
+
+<style>

• p {font-size: larger}
• span {font-size: inherit}

Using something that isn't font-size would seem better here, given if the span rule isn't applied or the font-size: inherit ignored this test will still pass. Seems much better to use a property that isn’t inherited by default (border or display, maybe?).

I added a border as well.

How do I update the pull request?

In css21/cascade/inherit-computed-002-ref.html:

@@ -0,0 +1,11 @@
+
+
+<title>CSS Reference File</title>
+<link rel="author" title="Bert Bos" href="mailto:bert@w3.org"

Missing >

Fixed.

In css21/cascade/inherit-computed-002.html:

@@ -0,0 +1,14 @@
+
+
+<title>CSS cascading: Inherit the computed value</title>
+
+
+
+<link rel="author" title="Bert Bos" href="mailto:bert@w3.org"

Missing >

Fixed.

In css21/margin-padding-clear/margin-collapse-min-height-001.xht:

@@ -0,0 +1,26 @@
+
+
+
+<title>CSS margin: Margin collapsing and elements with min-height</title>
+
+
+
+
+<style type="text/css">

• body {min-width: 17em}
• div.parent {min-height: 2em; height: auto; margin: 1em 0}
• div.last-child {margin-bottom: 5em; position: relative}

This being position: relative doesn’t have an effect, does it?

Indeed, it doesn’t. Removed. (It was a copy-paste from another test that I forgot to delete.)

In css21/margin-padding-clear/margin-collapse-min-height-001.xht:

+
+
+
+<style type="text/css">

• body {min-width: 17em}
• div.parent {min-height: 2em; height: auto; margin: 1em 0}
• div.last-child {margin-bottom: 5em; position: relative}
• div.float {float: left}
  +</style>
  +

•  <div></div>
  

•  <div></div>
  

•  <div class="last-child">
  

• There's more above than below

I don't understand that statement. There's more… what, above? “There must be more whitespace above this sentence than there is between this and the horizontal rule"?

Yes, that’s what it should say and what it says now. (But you don’t have to understand the text: There is a reference file.)

https://www.w3.org/Bugs/Public/show_bug.cgi?id=16036 includes what seems like a better, easier to understand test of a more normal form (i.e., green boxes, no red). It’s much easier to distinguish no red than differing amounts of whitespace!

I failed to find an existing test. That one is nice, indeed.

I’ve added a version of that test and made a reference file.

In css21/visudet/height-computed-001.xht:

• <style type="text/css">
• body {background: white; color: black; font-size: 14px}
• span {display: inline-block}
• #container {background: green; height: 70px; min-height: 140px}
• #child1 {background: #66F; width: 70px; height: inherit} /* Inherits 70px */
• #child2 {background: #AAF; width: 70px; height: 70px}
• ]]></style>
• +

• Test passes if (1) the two blue rectangles have the same height

In general, we've used teal/orange boxes, rather than two of similar colour. (Likely this isn't that essential, but…)

I’d also recommend not having green there as a signifier, rather having a 140x70 red box under the other two, and then have the condition that there should be no red.

No change for now, but I’ll think about it.

In css21/syntax/escaped-url-001.xht:

@@ -0,0 +1,33 @@
+
+

• <title>CSS Test: Escapes in a URI token</title>

• <meta name="assert" content="URI {U}{R}{L}({w}{string}{w})|
• {U}{R}{L}({w}([!#$%&-[]-~]|{nonascii}|{escape}){w})" />

• <style type="text/css">
• div {background: red; width: 5em; height: 1em}

uri-015 tests much of this already, though not with the variation in case.

In css21/syntax/characters-0080-009F-001.xht:

@@ -0,0 +1,31 @@
+
+

• <title>CSS Test: Valid control characters in identifiers</title>

• <style type="text/css">
• /* Warning: the next lines contain control characters \201..\237 */

Given this is XHTML and not HTML, can we not just use character references for them?

Yes, but I don’t know if that poses problems for the conversion to HTML4.

In css21/syntax/support/.htaccess:

@@ -33,3 +33,8 @@ AddCharset koi8-r .css
<files ~ '^at-charset-07[1234567].css$'>
RemoveCharset .css

+
+

Please also add a character-encoding-041.css.headers file.

Added.

(But what does that file do?)

In css21/visudet/height-percentage-004.xht:

+"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+

• <title>CSS test: inherited percentage height</title>

• <style type="text/css">
• p {margin-right: 7em}
• div {height: 100%; background: red}
• div div {position: absolute; top: 0; right: 0; background: lightblue;

lightblue isn’t a colour in 2.2, suggest aqua instead.

Fixed.

In css21/visudet/height-percentage-004.xht:

@@ -0,0 +1,32 @@
+
+

• <title>CSS test: inherited percentage height</title>

This doesn’t exist in this commit (though it's added later in c90c4c4) if you care about having a clean history (it certainly makes reviewing easier!)

Sure, but how do I fix that? Cancel the pull request and make a new one?

In css21/visudet/height-percentage-004.xht:

• <style type="text/css">
• p {margin-right: 7em}
• div {height: 100%; background: red}
• div div {position: absolute; top: 0; right: 0; background: lightblue;

•  height: inherit}
  

  -</style>
• <style type="text/css">

•  p {margin-right: 7em}
  

•  #container {height: 100%; background: red}
  

•  #container div {position: absolute; top: 0; right: 0;
  

•    background: lightblue; height: inherit}
  

In case GitHub eats my earlier comment: lightblue isn’t a colour keyword in 2.2.

Seems Github didn’t eat it. :-) Fixed.

Bert

Bert Bos ( W 3 C ) http://www.w3.org/
http://www.w3.org/people/bos W3C/ERCIM
bert@w3.org 2004 Rt des Lucioles / BP 93
+33 (0)4 92 38 76 92 06902 Sophia Antipolis Cedex, France

[syncbot]

Automatic validation checks of commit a272e01 discovered the following problems:

In css21/fonts/font-045.xht:

• Line 10: Unknown anchor changes.html#q142 in specification link to css21.

In css21/margin-padding-clear/margin-collapse-min-height-002-ref.xht:

• Reference must not have specification links.
• Reference must not have assertions.

[syncbot]

Automatic validation checks of commit e1798ef discovered the following problem:

In css21/fonts/font-045.xht:

• Line 10: Unknown anchor changes.html#q142 in specification link to css21.

[syncbot]

Automatic validation checks of commit 7a4c19c discovered the following problem:

In css21/fonts/font-045.xht:

• Line 10: Unknown anchor changes.html#q142 in specification link to css21.

[bert-github]

On Tuesday, December 6, 2016 10:10:32 AM CET Greg Whitworth wrote: @bert-github Hey, some of the tests are pointing to CSS2.1 version of the spec. Are these updates done to the 2.1 version or the 2.2? If they're the 2.2 it would be good if the spec linked to that one.

Yes, I just changed the links to point to CSS 2.2 instead. I will also add links to point to the Changes section of CSS 2.2. Bert -- Bert Bos ( W 3 C ) http://www.w3.org/ http://www.w3.org/people/bos W3C/ERCIM bert@w3.org 2004 Rt des Lucioles / BP 93 +33 (0)4 92 38 76 92 06902 Sophia Antipolis Cedex, France

[gregwhitworth]

Just a general request, not sure what the precedent is since this 2.1 stuff, but can the 2.2 tests live in a 2.2 folder somewhere rather than them being intermixed with 2.1?

[syncbot]

Automatic validation checks of commit 15fb17d discovered the following problem:

In css21/fonts/font-045.xht:

• Line 10: Unknown anchor changes.html#q142 in specification link to css21.

[gsnedders]

@gregwhitworth that gets into the whole question of whether we want to maintain a 2.0, 2.1, 2.2, etc. testsuite and then whether we have a testsuite for 2.1+errata separately to 2.1 (which in principle should be the same to 2.2, AIUI); thus far we've always leaned towards having a single 2.x testsuite which happens to currently live in css21 directory (hence why we have no tests for the 2.0 definition of display: run-in for example).

[gregwhitworth]

Minor: We should probably ensure (and I haven't checked in the CSSOM or equivalent) that the computed statement is true. This is testing the overall layout, but not the computed value that is returned, at first glance I'm not seeing one. But maybe @gsnedders or @SimonSapin know of one.

[gregwhitworth]

I'm not seeing one at first glance, but do we have a test that tests this statement?

Negative values for 'min-height' and 'max-height' are illegal.

[gregwhitworth]

Thanks for getting a test in for this!! :)

[gsnedders]

@bert-github:

This doesn’t exist in this commit (though it's added later in c90c4c4) if you care about having a clean history (it certainly makes reviewing easier!)

Sure, but how do I fix that? Cancel the pull request and make a new one?

So, to answer that, for future reference: a pull request is from a branch to another branch, so the pull request is whatever you push to that branch (even if it's a non-fast-forward push that needs to be forced).

In general, when fixing an issue introduced by commit 0000000, the best thing to do when fixing it is to use git commit --fixup=0000000, which automatically sets the first line of the commit message to fixup! followed by commit 0000000's first line; if you want to add more in the way of a description feel free to add further lines here.

After review and everything, you can use git rebase -i --autosquash (probably with the upstream branch as an argument, or something like git merge-base HEAD origin/master in backticks), which will "squash" everything down into the original commit (so the fixups disappear).

The git-rebase manpage is pretty decent, as is https://github.com/servo/servo/wiki/Beginner's-guide-to-rebasing-and-squashing.

[gsnedders]

Landed in #1250.