- From: Domenic Denicola <notifications@github.com>
- Date: Fri, 01 Dec 2017 01:35:32 +0000 (UTC)
- To: w3c/payment-request <payment-request@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <w3c/payment-request/pull/654/review/80384073@github.com>
domenic requested changes on this pull request.
Found some nits!
> + <dt>
+ <dfn>Sorting code</dfn>
+ </dt>
+ <dd>
+ The sorting code as used in, for example, France.
+ </dd>
+ <dt>
+ <dfn>Language code</dfn>
+ </dt>
+ <dd>
+ The language in which the address is provided. It's used to determine
+ the field separators and the order of fields when formatting the
+ address for display.
+ </dd>
+ <dt>
+ <dfn>organization</dfn>
The capitalization is inconsistent on these components of an address. I think consistently lowercase would be best.
> </h2>
+ <pre class="idl">
+ [SecureContext, Exposed=(Window,Worker), Constructor(optional AddressInit details)]
Does exposing this in workers make sense?
> </h2>
+ <pre class="idl">
+ [SecureContext, Exposed=(Window,Worker), Constructor(optional AddressInit details)]
+ interface PaymentAddress {
+ [Default] object toJSON();
+ readonly attribute DOMString country;
The order here doesn't match the above, which is pretty confusing.
> - To maintain user's privacy, implementers need to be mindful
- that a shipping address's associated phone number might be
- different or the same from that of the end-user's. As such,
- implementers need to take care to not provide the end user's
- phone number to the merchant without the end user's consent.
- </p>
- </aside>
- </li>
- <li>Set <var>address</var>.<a>[[\languageCode]]</a> to a
+ <section>
+ <h2>
+ Constructor
+ </h2>
+ <p>
+ The steps to <dfn data-lt="PaymentAddress.PaymentAddress()"
+ data-no-default="">construct a <code>PymentAddress</code></dfn>
Typo "PymentAddress"
> + <h2>
+ Constructor
+ </h2>
+ <p>
+ The steps to <dfn data-lt="PaymentAddress.PaymentAddress()"
+ data-no-default="">construct a <code>PymentAddress</code></dfn>
+ with <a>AddressInit</a> <var>details</var> are given by the
+ following algorithm:
+ </p>
+ <ol data-link-for="AddressInit">
+ <li>Let <var>address</var> be a new instance of
+ <a>PaymentAddress</a> with <a>[[\addressLine]]</a> set to an empty
+ <code><a data-cite=
+ "!WEBIDL#idl-sequence">sequence</a><<a data-cite=
+ "WEBIDL#idl-DOMString">DOMString</a>></code>, and all other
+ internal slots set to the empty string.
Do you want to distinguish between null and empty string? Null feels a bit more natural for "unset" fields.
I have a feeling we discussed this before... apologies if so.
> + "!INFRA#ascii-uppercase">ASCII uppercasing</a>
+ <var>details</var>["<a>country</a>"].
+ </li>
+ <li>If <var>country</var> is not a valid [[!ISO3166]] alpha-2
+ code, throw a <a>RangeError</a>.
+ </li>
+ </ol>
+ </li>
+ <li>If <var>details</var>["<a>addressLine</a>"] is present:
+ <ol>
+ <li>Push each item of <var>details</var>["<a>addressLine</a>"]
+ in order into <var>address</var>.<a>[[\addressLine]]</a>.
+ </li>
+ </ol>
+ </li>
+ <li>Freeze <var>details</var>["<a>addressLine</a>"].
You don't want to freeze the incoming dictionary. I think instead you want to set address.[[addressLine]] to a new frozen array created from details["addressLine"]. (In place of both of the above steps.)
> + <li>If <var>country</var> is not a valid [[!ISO3166]] alpha-2
+ code, throw a <a>RangeError</a>.
+ </li>
+ </ol>
+ </li>
+ <li>If <var>details</var>["<a>addressLine</a>"] is present:
+ <ol>
+ <li>Push each item of <var>details</var>["<a>addressLine</a>"]
+ in order into <var>address</var>.<a>[[\addressLine]]</a>.
+ </li>
+ </ol>
+ </li>
+ <li>Freeze <var>details</var>["<a>addressLine</a>"].
+ </li>
+ <li>If <var>details</var>["<a>region</a>"] is present, set
+ <var>address</var>.<a>[[\region]]</a> to the result of trimming
What is "trimming"? :) I think you want https://infra.spec.whatwg.org/#strip-leading-and-trailing-ascii-whitespace
> + <li>Push each item of <var>details</var>["<a>addressLine</a>"]
+ in order into <var>address</var>.<a>[[\addressLine]]</a>.
+ </li>
+ </ol>
+ </li>
+ <li>Freeze <var>details</var>["<a>addressLine</a>"].
+ </li>
+ <li>If <var>details</var>["<a>region</a>"] is present, set
+ <var>address</var>.<a>[[\region]]</a> to the result of trimming
+ <var>details</var>["<a>region</a>"]
+ </li>
+ <li>If <var>details</var>["<a>phone</a>"] is present:
+ <ol>
+ <li>Check if <var>details</var>["<a>phone</a>"] is a
+ <a>structurally valid phone number</a>, throw a
+ <a>RangeError</a>.
Presumably only throw the RangeError if it is not a structurally valid phone number. As phrased, this always throws it.
> + </ol>
+ </li>
+ <li>Freeze <var>details</var>["<a>addressLine</a>"].
+ </li>
+ <li>If <var>details</var>["<a>region</a>"] is present, set
+ <var>address</var>.<a>[[\region]]</a> to the result of trimming
+ <var>details</var>["<a>region</a>"]
+ </li>
+ <li>If <var>details</var>["<a>phone</a>"] is present:
+ <ol>
+ <li>Check if <var>details</var>["<a>phone</a>"] is a
+ <a>structurally valid phone number</a>, throw a
+ <a>RangeError</a>.
+ </li>
+ <li>Set <var>address</var>.<a>[[\phone]]</a> to the result of
+ <a>canonicalize a phone number</a>
canonicalizing the phone number
> + <table>
+ <tr>
+ <th>
+ Internal slot
+ </th>
+ <th>
+ Description (<em>non-normative</em>)
+ </th>
+ </tr>
+ <tr>
+ <td>
+ <dfn>[[\country]]</dfn>
+ </td>
+ <td>
+ A <a>country</a> as an [[!ISO3166]] alpha-2 code or the empty
+ string. The canonical form is upper case. For example, "JP".
I would phrase this as "stored in its canonical uppercase form"
> + Structurally valid phone numbers
+ </h3>
+ <p>
+ The steps to check if a <a data-cite=
+ "WEBIDL#idl-DOMString">DOMString</a> <var>tel</var> is
+ <dfn>structurally valid phone number</dfn> is given by the
+ following algorithm.
+ </p>
+ <p class="note">
+ The algorithm checks that a phone number conforms to [[!E.164]].
+ </p>
+ <ol class="algorithm">
+ <li>If <var>tel</var> contains U+000A LINE FEED (LF) or U+000D
+ CARRIAGE RETURN (CR) characters, return false.
+ </li>
+ <li>Strip and collapse ASCII whitespace in <var>tel</var>.
Link to https://infra.spec.whatwg.org/#strip-and-collapse-ascii-whitespace
> + <p>
+ The steps to check if a <a data-cite=
+ "WEBIDL#idl-DOMString">DOMString</a> <var>tel</var> is
+ <dfn>structurally valid phone number</dfn> is given by the
+ following algorithm.
+ </p>
+ <p class="note">
+ The algorithm checks that a phone number conforms to [[!E.164]].
+ </p>
+ <ol class="algorithm">
+ <li>If <var>tel</var> contains U+000A LINE FEED (LF) or U+000D
+ CARRIAGE RETURN (CR) characters, return false.
+ </li>
+ <li>Strip and collapse ASCII whitespace in <var>tel</var>.
+ </li>
+ <li>If the length is greater 15, return false.
If **_tel_'s** length is greater **than** 15...
--
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/payment-request/pull/654#pullrequestreview-80384073
Received on Friday, 1 December 2017 01:35:57 UTC