Re: [w3c/manifest] Add IDL section and define IDL for all the members (#613)

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