- From: Marcos Cáceres <notifications@github.com>
- Date: Fri, 22 Sep 2017 05:49:57 +0000 (UTC)
- To: w3c/manifest <manifest@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <w3c/manifest/pull/613/review/64484990@github.com>
marcoscaceres requested changes on this pull request.
> @@ -1261,7 +1259,7 @@ <h3 id="navigation-scope-security-considerations">
</p>
<p>
A <a>manifest</a> is obtained and applied regardless of the
- <a><code>media</code></a> attribute of the <a><code>link</code>
+ <a>media</a> attribute of the <a><code>link</code>
Nit: seems you missed a closing `</a>`. Also, please remove the `<code>`.
> @@ -1574,7 +1572,7 @@ <h3 id="applying">
<div>
<p>
Please note that the <a>start URL</a> is not necessarily the value
- of the <a><code>start_url</code> member</a>: the user or user agent
+ of the <a data-link-for="WebAppManifest"><code>start_url</code></a> member: the user or user agent
Bit cleaner, you can do:
```HTML
<a data-tl="WebAppManifest.start_url">start_url</a>
```
> @@ -1592,17 +1590,39 @@ <h3 id="applying">
<div class="issue" data-number="446"></div>
</section>
</section>
- <section>
+ <section data-dfn-for="WebAppManifest" data-link-for="WebAppManifest">
<h2>
Manifest and its members
This should change to:
```HTML
<h2>
<dfn>WebAppManifest</dfn> dictionary
</h2>
```
> <h2>
Manifest and its members
</h2>
<p>
A <dfn>manifest</dfn> is a JSON document that contains startup
parameters and application defaults for when a web application is
- launched. A manifest consists of a top-level <a>object</a> that
- contains zero or more members. Each of the members are defined below,
- as well as how their values are processed.
+ launched. A manifest consists of a top-level <dfn>WebAppManifest</dfn>
See comment above... so here ` <a>WebAppManifest</a>`
> <h2>
Manifest and its members
</h2>
<p>
A <dfn>manifest</dfn> is a JSON document that contains startup
parameters and application defaults for when a web application is
- launched. A manifest consists of a top-level <a>object</a> that
- contains zero or more members. Each of the members are defined below,
- as well as how their values are processed.
+ launched. A manifest consists of a top-level <dfn>WebAppManifest</dfn>
Actually, " A manifest consists of a top-level..." is redundant... remove this sentence. The WebIDL itself already says this.
> </p>
+ <pre class="idl">
Nit: I prefer seeing the IDL before the description... "an IDL block speaks a thousand words" 😂
> @@ -1663,7 +1686,7 @@ <h3 id="applying">
end-user, the user agent MUST use the <a>base direction</a> to
compute directional runs and layout or position text correctly in
text containing mixed-direction sequences [[!BIDI]]. When the <a>base
- direction</a> is "<a data-lt="direction auto">auto</a>" the user
+ direction</a> is "<a data-link-for="TextDirectionType">auto</a>" the user
move `data-link-for="TextDirectionType"` inside ` <dl data-dfn-for="TextDirectionType">`
> @@ -2090,15 +2121,15 @@ <h3 id="applying">
<code>orientation</code> member
</h3>
<p>
- The <dfn id="member-orientation"><code>orientation</code>
- member</dfn> is a <a>string</a> that serves as the <a>default
+ The <dfn>orientation</dfn>
Nit: the `<dfn>orientation</dfn>` and so on, should all be defined inside the `h3`s... like this:
```HTML
<h3>
<dfn>orientation</dfn> member
</h3>
<p>The <a>orientation</a> member bla bla..</p>
```
Applies to all of the ones below...
> @@ -2995,13 +3035,13 @@ <h3 id="applying">
</pre>
</div>
</section>
- <section>
+ <section data-dfn-for="imageresource">
```
<section data-dfn-for="ImageResource" data-link-for="ImageResource">
```
> @@ -3094,18 +3134,18 @@ <h3 id="applying">
</li>
</ol>
</section>
- <section>
+ <section data-dfn-for="imageresource">
You shouldn't need:
```
data-dfn-for="imageresource"
```
ReSpec will find it from the parent section.
> @@ -3234,41 +3274,40 @@ <h3 id="applying">
Each member specifies which object a multi-purpose member can be used
with.
</p>
- <section>
+ <section data-dfn-for="ExternalApplicationResource" data-link-for="ExternalApplicationResource">
`platform` should be defined in the ExternalApplicationResource section. If this is a shared definition, then we should make a note that two things share the same definition.
> @@ -3472,11 +3517,20 @@ <h3 id="applying">
</section>
<section>
Add:
`data-dfn-for= "ExternalApplicationResource"` and `data-link-for="ExternalApplicationResource"`
> @@ -3472,11 +3517,20 @@ <h3 id="applying">
</section>
<section>
Please make sure all sections that define IDL things have both.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/manifest/pull/613#pullrequestreview-64484990
Received on Friday, 22 September 2017 05:50:22 UTC