- From: Gérard Talbot <css21testsuite@gtalbot.org>
- Date: Sat, 08 Nov 2014 22:03:50 -0500
- To: Manuel Rego Casasnovas <rego@igalia.com>
- Cc: Public CSS Test suite mailing list <public-css-testsuite@w3.org>
Le 2014-11-03 11:51, Manuel Rego Casasnovas a écrit : > Fixing the subject. Sorry :-( > > On 03/11/14 17:47, Manuel Rego Casasnovas wrote: >> Hi Gérard, >> >> here are my first 3 tests checking the new values "grid" and >> "inline-grid" for "display" property. >> >> I've joint them in a single pull request but I can split it in 3 >> different pull-requests as you wish. >> >> Pull request: https://github.com/w3c/csswg-test/pull/626 >> >> Tests sources: >> * >> https://github.com/Igalia/csswg-test/blob/css-grid-display-tests/css-grid-1/grid-model/display-grid.xht Filename should be following the file name format test-topic-###.ext : " The file name format is test-topic-###.ext where test- topic somewhat describes the test and ### is a zero-filled number used to keep the file names unique. " http://testthewebforward.org/docs/css-naming.html So here, I would suggest to filename-rename your test as grid-display-001.xht or as display-grid-001.xht or as grid-display-grid-001.xht As a test creator, you want to achieve an unique and somewhat descriptive filename. In order to achive this, you may have to prefix your filename with the most significant name of the spec. Eg. All the tests in http://test.csswg.org/source/css-multicol-1/ are prefixed with "multicol" so that there can not be a filename conflict with any tests of any other spec, including (especially) CSS2.1. ----- line 4: <meta charset="utf-8" /> Over time, the HTML5 template was created and then there was some mixup with the XHTML 1 template. <meta charset="utf-8"> is for HTML5. The original, initial XHTML 1 template did not require to declare a character set because character encoding of the XHTML document is by default UTF-8...which is the character encoding that should be using all tests... otherwise you would declare the character encoding into an xml declaration. If you decide to create your tests in XHTML1, then I suggest to install HTML Validator 0.9.5.8 (extension available for Firefox only at http://users.skynet.be/mgueury/mozilla/ ) which can run offline and will report any validation error with your tests. ----- line 5: <title>CSS Grid Layout: 'grid' value for 'display' property</title> I suggest <title>CSS Grid Layout Test: 'grid' value for 'display' property</title> ----- line 7: <link rel="help" href="http://www.w3.org/TR/css-grid-1/#grid-containers" /> If there is more than 1 help link, then it is preferable to specify each link with the title attribute with its section description. Like: <link rel="help" href="http://www.w3.org/TR/css-grid-1/#grid-containers" title="3.1 Establishing Grid Containers" /> Some people like myself use a document relation bar extension and we can read the title of such link in our browser. ----- line 10: <style> If you create a test in HTML5, then <style> is correct. If you create a test in XHTML1, then use <style type="text/css"><![CDATA[ (...) ]]></style> The <![CDATA[ and ]]> chunks are solely for understanding and managing adequately, accordingly <, > and & which could be inside the CSS code (child selector) or javascript code (logical AND). More info: http://www.w3.org/TR/xhtml1/#h-4.8 ----- line 19: .grid-overlapping-green If you use "reference" in an identifier, then I would normally expect its correspondent counter-part in the test to use "test". So, I suggest line 10 #reference-overlapped-red line 19 .test-overlapping-green What you did was already good. ----- Nitpicking "For indentation, spaces are preferred over tabs." http://testthewebforward.org/docs/review-checklist.html ----- With GitHub, I can not view the test as rendered in a browser (don't know how; I had to copy and save the code into an HTML editor and then open it in a browser ... not convenient!) and I can not verify things like trailing whitespace (whitespace at the end of lines) or Unicode byte order mark (BOM U+FEFF) (Shepherd is able to spot and report this, btw) and usage of Unix line endings. >> * >> https://github.com/Igalia/csswg-test/blob/css-grid-display-tests/css-grid-1/grid-model/display-inline-grid.xht On top of issues spotted in your first test, I suggest to filename-rename this test as grid-display-002.xht or display-grid-002.xht or grid-display-inline-grid-001.xht >> * >> https://github.com/Igalia/csswg-test/blob/css-grid-display-tests/css-grid-1/grid-model/parsing-display-grid-inline.xht On top of issues spotted in your first test,... lines 10, 11 and 34: <script src="/resources/testharness.js"></script> <script src="/resources/testharnessreport.js"></script> <script> type="text/javascript" is required and should be declared in XHTML1 document but not in HTML5. Addendum ******** For a reason I do not understand, locally, I must edit <script type="text/javascript" src="./resources/testharness.js"></script> <script type="text/javascript" src="./resources/testharnessreport.js"></script> because this linkage <script type="text/javascript" src="/resources/testharness.js"></script> <script type="text/javascript" src="/resources/testharnessreport.js"></script> will not work. ----- line 39: You may want to do a "search and replace" in your file of the string testComptuedStyleDisplay with testComputedStyleDisplay Gérard >> >> Thanks, >> Rego -- Test Format Guidelines http://testthewebforward.org/docs/test-format-guidelines.html Test Style Guidelines http://testthewebforward.org/docs/test-style-guidelines.html Test Templates http://testthewebforward.org/docs/test-templates.html CSS Naming Guidelines http://testthewebforward.org/docs/css-naming.html Test Review Checklist http://testthewebforward.org/docs/review-checklist.html CSS Metadata http://testthewebforward.org/docs/css-metadata.html
Received on Sunday, 9 November 2014 03:04:28 UTC