Re: [w3c/webpayments-payment-apps-api] Respec fixes (#126)

marcoscaceres requested changes on this pull request.



> @@ -179,8 +182,24 @@
       settings object</dfn> are defined by [[!HTML5]].</dd>
       <dt>ECMA-262 6th Edition, The ECMAScript 2015 Language
       Specification</dt>
-      <dd>The term <dfn>Promise</dfn> is defined by
-      [[!ECMA-262-2015]].</dd>
+      <dd>
+          The terms <dfn data-cite=
+          "!ECMA-262-2015#sec-promise-objects">Promise</dfn>, <dfn data-cite=
+          "!ECMA-262-2015#sec-object-internal-methods-and-internal-slots">internal
+          slot</dfn>, <code><dfn data-cite=
+          "!ECMA-262-2015#sec-native-error-types-used-in-this-standard-typeerror">
+          TypeError</dfn></code>, and <code><dfn data-cite=
+          "!ECMA-262-2015#sec-json.stringify">JSON.stringify</dfn></code> are
+          defined by [[!ECMA-262-2015]].
+        <p>

Just reference PR spec directly here. Don't redefine it. 

> @@ -290,10 +309,10 @@
       displaying and grouping Instruments according to information
       (labels and icons) provided at registration or otherwise
       available from the Web app.</li>
-      <li>When the user selects an Instrument, the user agent fires the
-      paymentrequest event in the service worker whose registration
+      <li>When the user (the <dfn>payer</dfn>) selects an Instrument, the user agent fires the

this should link to "fire an event" (DOM spec) and needs to use the "user interaction task source" using the "PaymentRequestEvent" interface. 

> @@ -366,8 +384,8 @@
         users by the browser for selection.</dd>
       </dl>
     </section>
-    <section id="payment-instruments">
-      <h2><a>PaymentInstruments</a> interface</h2>
+    <section data-dfn-for="PaymentInstruments">

Also add `data-link-for="PaymentInstruments"` - same for other sections. Saves some work later. 

> @@ -479,9 +497,9 @@
 
 
     </section>
-    <section id="payment-instrument">
-      <h2><a>PaymentInstrument</a> dictionary</h2>
-      <pre id="payment-instrumnt-idl" class="idl">
+    <section data-dfn-for="PaymentInstrument">
+      <h3><dfn>PaymentInstrument</dfn> dictionary</h3>
+      <pre id="payment-instrument-idl" class="idl">

Please don't ID `pre` sections.  

> @@ -490,25 +508,23 @@
       };
       </pre>
       <dl>
-        <dt><code>name</code> member</dt>
+        <dt><dfn id="payment-instrument-name">name</dfn> member</dt>
         <dd>The <code>name</code> member is a string that

Please replace this with an `<a>`

>          <dd>The <code>icons</code> member is an array of image
         objects that can serve as iconic representations of the
         payment Instrument when presented to the user for
         selection.</dd>
-        <dt><code>enabledMethods</code> member</dt>
+        <dt><dfn id="payment-instrument-enabled-methods">enabledMethods</dfn> member</dt>

Please remove this id

>          <dd>The <code>icons</code> member is an array of image
         objects that can serve as iconic representations of the
         payment Instrument when presented to the user for
         selection.</dd>
-        <dt><code>enabledMethods</code> member</dt>
+        <dt><dfn id="payment-instrument-enabled-methods">enabledMethods</dfn> member</dt>

Same with any other id on DFN. Let ReSpec take care of that. 

>            member lists the <a>payment method identifiers</a> of the
           payment methods enabled by this Instrument.
         </dd>
-        <dt><code>capabilities</code> member</dt>
-        <dd>The <dfn id=
-        "payment-instrument-capabilities"><code>capabilities</code></dfn>
+        <dt><dfn id="payment-instrument-capabilities">capabilities</dfn> member</dt>

As above. 

>            member lists the <a>payment method identifiers</a> of the
           payment methods enabled by this Instrument.
         </dd>
-        <dt><code>capabilities</code> member</dt>
-        <dd>The <dfn id=
-        "payment-instrument-capabilities"><code>capabilities</code></dfn>
+        <dt><dfn id="payment-instrument-capabilities">capabilities</dfn> member</dt>
+        <dd>The <code>capabilities</code>

Please avoid `code`. Let ReSpec determine what should be code or not from the dfn. Just put `<a>` here. 

>            Promise&lt;sequence&lt;DOMString&gt;&gt;  keys();
           Promise&lt;boolean&gt;              has(DOMString walletKey);
           Promise&lt;void&gt;                 set(DOMString walletKey,
-                                              WalletDetails details);
+                                              PaymentWallet details);

Move this to the previous line. 

> @@ -568,29 +584,29 @@
           </ol>
         </dd>
 
-        <dt><code>keys</code> method</dt>
+        <dt><dfn>keys</dfn> method</dt>

Method definitions should really be in their own section - same with attributes. See PR spec. Otherwise, they don't show up in the ToC, making them difficult to find. 

>          <dd>
           When called, this method executes the following steps:
           <ol>
             <li> Let <var>p</var> be a new promise.</li>
             <li> Run the following steps in parallel:</li>
             <ol>
               <li>Resolve <var>p</var> with a sequence that contains all
-              the <code>walletKey</code>s for the <a>WalletDetails</a>s
+              the <code>walletKey</code>s for the <a>PaymentWallet</a>s

Change code to `<a>` 

> @@ -622,28 +638,28 @@
       </dl>
 
     </section>
-    <section id="wallet-details">
-      <h2><a>WalletDetails</a> dictionary</h2>
-      <pre id="wallet-details-idl" class="idl">
-      dictionary WalletDetails {
+    <section data-dfn-for="PaymentWallet">
+      <h3><dfn>PaymentWallet</dfn> dictionary</h3>

BTW, seems odd that these are H3? These should really be H2... there might be a structuring problem with the spec.

> @@ -622,28 +638,28 @@
       </dl>
 
     </section>
-    <section id="wallet-details">
-      <h2><a>WalletDetails</a> dictionary</h2>
-      <pre id="wallet-details-idl" class="idl">
-      dictionary WalletDetails {
+    <section data-dfn-for="PaymentWallet">
+      <h3><dfn>PaymentWallet</dfn> dictionary</h3>
+      <pre id="payment-wallet-idl" class="idl">

Remove ID.

>            required DOMString name;
           sequence&lt;ImageObject&gt; icons;
           required sequence&lt;DOMString&gt; instrumentKeys;
       };
       </pre>
       <dl>
-        <dt><code>name</code> member</dt>
+        <dt><dfn id="payment-wallet-name">name</dfn> member</dt>

Remove all these ids... 

> @@ -661,7 +677,7 @@
       Issue 94</a>. The means for code requesting permission to handle
       payments is not yet defined. The code below is based on one
       potential model (a <code>requestPermission()</code> method on the
-      <a>PaymentManager</a>),
+      <code>PaymentManager</code>),

`a` here too 

> @@ -1130,7 +1144,7 @@
                 which does not bubble, cannot be canceled, and has
                 no default action.
                 </li>
-                <li>Set the <a>appRequest</a> attribute of
+                <li>Set the <a href="#apprequest">appRequest</a> attribute of

Don't link directly to things like this. Just use `<a data-lt="PaymentAppRequest .appRequest">appRequest</a>`

> @@ -1224,23 +1238,23 @@
               <var>promise</var> with that exception and abort
               these steps.</li>
               <li>If the origin of <var>newContext</var> is not the same as the
-              <var>event</var>.<a>appRequest</a>.<a href=
+              <var>event</var>.<a href="#appRequest">appRequest</a>.<a href=

Same here.

-- 
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/webpayments-payment-apps-api/pull/126#pullrequestreview-31199266

Received on Thursday, 6 April 2017 00:20:00 UTC