Re: [web-nfc] Review fixes for #44.

I think you can merge right away, and handle fixes in subsequent PRs.

Comments follow:

   - In
   https://rawgit.com/zolkis/web-nfc/gh-pages/index.html#the-watch-method:
      - "Browsing contexts" (
      https://html.spec.whatwg.org/multipage/browsers.html#browsing-context)
      don't have origins. They have an active document that has an
origin, or you
      can talk about the incumbent settings object (
      https://html.spec.whatwg.org/multipage/webappapis.html#incumbent-settings-object),
      which also has an origin. This appears in postMessage() too.
      - "If the first argument options contains invalid values" doesn't say
      what makes a value "invalid". You need to link to something that
specifies
      that.
      - Usually, WebIDL will check the types of arguments for you, for
      example in https://heycam.github.io/webidl/#es-callback-function for
      the MessageCallback parameter.
      - "If the browsing context loses focus" doesn't really fit in the
      watch() algorithm. There should be another section that does things when
      focus is lost and regained.
   - In
   https://rawgit.com/zolkis/web-nfc/gh-pages/index.html#the-nfcwatchoptions-dictionary
      - Once you've given default values in the dictionary, you don't need
      to also give them in prose. e.g. you can remove "The default
value is null."
   - You should probably refer to http://www.w3.org/TR/page-visibility/
   when defining "focus". Do you really mean "focus"? If I click on my text
   editor or NFC debugger but can still see the NFC-using page, should it lose
   the ability to use NFC?
   - Does "trusted integrity NFC content" ever exist? How would a browser
   identify it? If we don't have a way to identify it, it probably shouldn't
   be in the spec.
   - In
   https://rawgit.com/zolkis/web-nfc/gh-pages/index.html#data-representation
   :
      - Saying "The NFCRecord.kind property represents the NFCRecordType…"
      is redundant with the IDL that declares kind as having the type
      NFCRecordType.
      - The "valid NFCRecord" definition is mostly redundant with the IDL
      definition of NFCRecord. You can remove the assertion on
'record.kind', and
      you should probably replace the assertion on record.data with a more
      precise union type for that field. "(DOMString or unrestricted double or
      object or BufferSource)?" should do it.
   - In
   https://rawgit.com/zolkis/web-nfc/gh-pages/index.html#dfn-receive-an-ndef-message
   :
   - "When the UA is to receive an NDEF message …" isn't quite the right
      phrasing for handling an event that comes in from outside the UA. Maybe
      "When the UA receives an NDEF message …".
      - Using "If the top-level browsing context loses focus" as step 1 of
      this algorithm is a little weird. Do you mean to say that if the browsing
      context loses focus during the execution of this algorithm, the algorithm
      needs to be paused until focus comes back, or that if the
browsing context
      doesn't have focus when the algorithm starts, the steps should just be
      aborted?
      - "If options.url is defined" will always be true,
      because NFCWatchOptions gives it a default value.
      - I don't see any enforcement that pages only receive NDEF messages
      from their own origin. Where is it? I suspect once this enforcement is in
      place, you won't need to check for "https" in the Web NFC Id, since the
      "secure context" restriction will handle it for you.
      - In the urn:nfc:ext:w3.org:webnfc branch, you probably need to
      continue to the next record, to avoid adding the uninitialized record to
      `message`.
      - These sub-steps should all be ordered lists, not bullets.
   - https://rawgit.com/zolkis/web-nfc/gh-pages/index.html#URL_pattern
   should mention whether "https://*mydomain.com/" is a valid pattern, or
   whether the "*" has to appear right before a ".". I feel like we had
   security problems in Chrome Extensions when we allowed "*" to be part of a
   host component, and it's not allowed anymore (
   https://developer.chrome.com/extensions/match_patterns), but I don't
   remember exactly what the problems were.
   - In
   https://rawgit.com/zolkis/web-nfc/gh-pages/index.html#the-pushmessage-method
   :
      - You don't need to say "If the options parameter is undefined, then
      let options be a new NFCPushOptions object with the default
values." WebIDL
      does that for you.
      - "On any termination of this algorithm, output must be cleared." is
      odd. `output` is a local variable, so of course it gets a fresh value on
      any execution of this algorithm.
      - A record.kind of "empty" silently throws away all the other
      records? That seems surprising.
      - The "When an NFC device comes within communication range" steps
      don't actually say to transfer any data.
      - "the sending MAY only happen if an NFC tag is tapped" means the
      sending can happen if an NFC peer is tapped, at the discretion of the UA.
      Is that what you mean?
      - Steps "If timer exists" through "If the top-level browsing context
      loses focus" should be phrased as "Wait until one of the following events
      happens: <sublist>" since they don't happen in sequence.


On Tue, Sep 15, 2015 at 8:53 AM, Anssi Kostiainen via GitHub <
sysbot+gh@w3.org> wrote:

> @jyasskin The editors believe [your feedback][1] has now been
> addressed. Could you please review this PR, and let @zolkis and
> @kenchris know if you'd like the editors to do further changes before
> merging. This version of the spec does not need to be perfect, but it
> should be a good base for the next iteration of the experimental
> implementation.
>
> For convenience, here's the [RawGit HTML preview of the head of this
> PR][2].
>
> Thank you again for your extensive and detailed feedback.
>
> [1]:
> https://lists.w3.org/Archives/Public/public-web-nfc/2015Sep/0033.html
> [2]: https://rawgit.com/zolkis/web-nfc/gh-pages/index.html
>
> --
> GitHub Notif of comment by anssiko
> See https://github.com/w3c/web-nfc/pull/46#issuecomment-140439333
>
>

Received on Wednesday, 16 September 2015 19:54:48 UTC