- From: L. David Baron <notifications@github.com>
- Date: Fri, 19 Apr 2019 16:06:57 -0700
- To: w3ctag/design-reviews <design-reviews@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <w3ctag/design-reviews/issues/344/485034489@github.com>
At this point I've read the main part of the specification, but not the appendices. (I'm not sure if I'll go through the appendices in detail.) A bunch of detailed comments follow. However, first, more generally, I'd note that many of the things that seemed likely to be scary as a result of reading the requirements document seem a lot less scary after reading the specification. It seems like the requirements document has gotten a bit out of sync with the specification, and it may be worth updating it to reflect how those requirements are addressed in the specification, or whether they are. (For example, the specification doesn't seem to have any notion of packages in it, whereas I was expected a packaging mechanism from the requirements document.) # PART I: Publication Manifest It's a little disturbing to see [section 2.3](https://w3c.github.io/wpub/#webidl) refer to WebIDL as a representation of a data structure, rather than as an API for accessing the representation of a data structure. The repeated use of WebIDL "partial" to amend the same interface within a single specification feels like a bad practice; it makes it hard to understand the interface. (I'd perhaps go so far as to say "partial" is always a bad practice.) The spec still seems short on information for how implementations / processors must interpret the manifest (in particular, a clear processing model -- such a model would explain how to handle, say, an @context that points to a different URL than it's supposed to). It seems like a decent amount may be expected based on, say the [section on additional properties in the manifest](https://w3c.github.io/wpub/#extensibility-manifest-properties) (although maybe it's not really expected since such processing is optional). dictionary LinkedResources should probably use the WebIDL "required" attribute on its required member. The description of default language and base direction seems a bit confusing. It seems to imply it's a default for the manifest and the publication, but then there's a note that says it only applies to the manifest and not to the resources that comprise the publication. (I also wonder whether LocalizableString should allow specifying base direction in line with https://w3c.github.io/string-meta/ . There's a note saying this isn't possible in JSON-LD, which makes me wonder whether the LocalizableString defined in this specification should really be a reference to something defined elsewhere.) The bit in the [accessibility report section](https://w3c.github.io/wpub/#accessibility-report) that uses an unregistered link rel value that looks like a URL seems odd. I don't know of any specification that defines URLs to be usable as link rel values; they're just keywords, so this seems worse than using a syntactically valid but not-yet-registered value, since it both doesn't fit the syntax and appears to imply to readers that they can use URLs in place of link rel values rather than registering them. (Although this isn't quite a link rel value, since the [definition of `LinkedResource`](https://w3c.github.io/wpub/#publication-link-def) says the rel value can be one of two things.) It seems best to just register the value. The same applies to [cover](https://w3c.github.io/wpub/#cover) nd [page list](https://w3c.github.io/wpub/#page-list). The [description of cover](https://w3c.github.io/wpub/#cover) says "If multiple covers are specified, each instance MUST define at least one unique property to allow user agents to determine its usability (e.g., a different format, height, width or relationship)." but doesn't say how to define a height or width. The section on [linked records](https://w3c.github.io/wpub/#extensibility-linked-records) endorses organizations other than IANA extending the set of rel values, which again seems a bit strange, although again what's being used here isn't strictly link rel values, but a superset of them defined earlier. # PART II: Web Publications [Publication Bounds](https://w3c.github.io/wpub/#wp-bounds) should perhaps refer to the concept of [URL Equivalence](https://url.spec.whatwg.org/#url-equivalence) in the URL spec. I noticed that [Primary Entry Page](https://w3c.github.io/wpub/#primary-entry-page) says that the primary entry page MUST be within the bounds of the publication (inferring that from the definition of the bounds), whereas [Address](https://w3c.github.io/wpub/#address) imposes the same requirement, using different words, as a SHOULD. It seems like these should match, or maybe one should just refer to the other. The words "identify the resource" in [3.3.4 Table of Contents](https://w3c.github.io/wpub/#table-of-contents) seem like thy should link to [3.4.1.7 Table of Contents](https://w3c.github.io/wpub/#wp-table-of-contents) but they actually link to 3.3.4. Same for the words "table of contents property definition". I don't immediately have any comments on [3.4.1.3 Canonical Identifier](https://w3c.github.io/wpub/#canonical-identifier), but I think it's interesting and there may be other TAG members who have comments on it. The second paragraph of [3.4.1.7 Table of Contents](https://w3c.github.io/wpub/#wp-table-of-contents) seems to incorrectly refer to "page list" rather than "table of contents"; this looks like a copy-and-paste error. Reading [3.5.1 Obtaining a Manifest](https://w3c.github.io/wpub/#wp-obtaining-manifest) made me wonder about how this is intended to integrate with CORS. I'd note that it doesn't quite seem to integrate with fetch properly, since "request" ought to link to [request](https://fetch.spec.whatwg.org/#concept-request), and that, in turn: * seems to require a **client** (there's no "unless otherwise stated")... although that seems odd to me; it's not quite clear to me how fetch integration is supposed to work. * recommends against using the `"no-cors"` mode It also ought to refer to the "credentials mode" rather than the "credentials". It's not clear to me whether the information from the manifest is going to be exposed to web content in ways that require CORS checks -- though it seems like it's probably a good idea to assume that it will at some point in the future, and thus that it's probably a good idea to use the "cors" mode. (I *think* that doesn't require special headers if the manifest is same-origin... although I'm not sure.) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/w3ctag/design-reviews/issues/344#issuecomment-485034489
Received on Friday, 19 April 2019 23:07:20 UTC