- From: Lachlan Hunt <lachlan.hunt@lachy.id.au>
- Date: Tue, 26 Jun 2007 02:03:40 +1000
- To: Philip Taylor <excors@gmail.com>
- CC: public-webapi@w3.org
Philip Taylor wrote:
>
> Some comments from reading through r1.23
> (<http://dev.w3.org/cvsweb/~checkout~/2006/webapi/selectors-api/Overview.html?rev=1.23&content-type=text/html;%20charset=utf-8>):
>
> Abstract: "[CSS21]" isn't italicised.
Fixed.
> "(often simply referred to as selector)" - the grammar sounds wrong;
> should it be "...as a selector"?
Fixed.
> "it's easier to match" - seems more conventional to say "it is"
I don't think it matters that much, but fixed anyway.
> instead. Also, ASCII ' vs some Unicode '-like symbol are used
> inconsistently throughout the document.
Oops, I thought I had already replaced all of those with U+2019
characters before. Fixed.
> "var cells = new Array();" - I'd say "var cells = [];" but maybe
> that's just personal preference.
Ok, fixed.
> I'd also remove the "var rows = null;" and put the "var" inside the
> loop to simplify the code, though perhaps that's a bad idea because
> JS scoping is weird.
Correct me if I'm wrong, but if it were declared inside the loop, the
variable would be destroyed and reallocated in memory for every loop.
By declaring it outside, it just allocates it once, which is more efficient.
> "as described in [RFC2119]." vs "is that from Selectors [Selectors]."
> - should be consistent on whether the citation is part of the
> sentence.
I fixed that one and another that I saw, though I will go through later
and ensure the whole spec follows the W3C Manual of Style on this issue.
http://www.w3.org/2001/06/manual/#References
> ""object implementing the Foo interface "." - stray space at the end.
Fixed.
> "return "http://example.org/test"" (and another couple of places in
> that section) - also probably personal preference, but I don't like
> dropping semicolons (except after '}'s).
Nor do I. Fixed.
> "var <var>x</var>" - seems odd to do that for just a few of the
> variables (and it's inconsistent with other variables like svgImages
> that are referred to in the non-code text.)
I decided to remove all <var> from the code samples, but leave them
where variables are referred to in the prose. That will be easier to
maintain.
> "if("test" == prefix)" - the lack of space before the '(' seems
> inconsistent with all the other code.
Fixed.
> The interface definitions have inconsistent indentation of the method
> names.
Fixed.
> "The selectAllElements() methods on the DocumentSelector interface
> ...." - says "in document order" twice.
Removed the second occurrence.
> Is it necessary to say that exceptions thrown inside
> lookupNamespaceURI must propagate outwards to the selectElement
> caller? Maybe that's obvious or is defined elsewhere.
AIUI, exceptions continue to propagate until they are caught or result
in an error. I don't believe there is a need for me to specify that in
this spec.
> Is it necessary to say what 'this' is, when nsresolver is a Function?
I don't know. I'd appreciate if someone who understands this issue
better than I do could advise me.
> "When the lookupNamespaceURI() method is invoked with an empty string
> as the argument ... it must do either of the following:" - that 'must'
> is redundant with the two following 'must's, which is perhaps
> irritating when you're trying to extract a unique testable assertion
> for each 'must' in the document.
I removed 'MUST' from the two alternatives.
> "User agents must handle prefixes case sensitively." - does 'handle'
> actually mean anything (to someone who knows more about this stuff
> than I do), given that it is already stated they must be passed
> case-preservingly to the NSResolver?
The intention of saying "preserving case" is to just make it clearer
that UAs shouldn't lowercase the prefix. The intention of saying "must
handle prefixes case sensitively" is to make it clear that UAs should
treat "foo" and "FOO" as distinct namespace prefixes. If you or anyone
can suggest reasonable alternative wording, I'll update the spec.
> "var lis = document.selectAllElements("ul.nav>li"); for (var i = 0; i
> < items.length; i++) {" - s/items/lis/, and for the next example.
Fixed.
> "[CaseMap] (Non-normative) Unicode Standard Annex #21 Case Mappings" -
> name isn't italicised.
Fixed.
> "M. Davis, editor, Unicode Consortium, March 2001." - s/,/./ in the middle.
Fixed.
> "[CSS21]: ... T. [funny symbols]elik" looks like double-encoded UTF8.
> (The [Selectors] copy of the same name is fine.)
Oops, I copied and pasted that back in from an older revision, without
first checking the encoding of the page I was copying from. Fixed.
--
Lachlan Hunt
http://lachy.id.au/
Received on Monday, 25 June 2007 16:03:56 UTC