NDEFReader.scan should say when it resolves and returns the promise p

[dbaron]

So reading through the examples in the explainer, one thing I wondered was that it seems a little odd to do the asynchronous await reader.scan() and then set the reader.onreading property to an event handler. It made me wonder whether the reading events could happen during the reader.scan() call. So I decided to look at the spec text for reader.scan() to understand whether this is the case -- and I ran into a problem. As far as I can tell, nothing in that algorithm ever resolves and returns p in the success case -- which seems to imply that the function never returns at all!

I think there should be something in that spec text that says to resolve p and return it in the success case. (At least I think that's how promise-using specifications are supposed to be written, although I suppose there actually isn't anything in the promises guide saying so.)

(I got here from w3ctag/design-reviews#461.)

[beaufortfrancois]

Thanks for raising this issue @dbaron!
I've fixed spec at #480

Regarding the onreading event listener added after await reader.scan(), we thought it makes sample code easier to read as resolved promise means both that access was granted and NFC is enabled.

[dbaron]

This still doesn't seem right, at least based on something that would make the examples work. Having "resolve p" as step 7 makes it sound like the promise doesn't get resolved until after the browser stops trying to detect NFC technology. I suspect the intent is to resolve the promise after "4. Add reader to the activated reader objects.", although it's possible that it could be right before (4) as well (which would be detectably different).

[dbaron]

(I'd also note that (2) and (3) don't really seem like separate steps. But it also seems like, implicitly, between them, there's something like "wait for this request to ???", so that they could be combined as something like: " If this is the first listener being set up, then make a request to all NFC adapters to listen to NDEF messages. Wait for this request to ???. If the request fails, then the UA MAY reject p with a "NotSupportedError" DOMException and abort these steps." (Though this also seems to forbid a UA from trying again if it fails once, which might be undesirable.)

Likewise, it's not clear to me that step 5, currently:

If the Document of the top-level browsing context is not visible (e.g. the user navigated to another page), then the registered activated reader objects still SHOULD continue to exist, but SHOULD become paused, i.e. the UA SHOULD NOT check and use them until the Document is visible again.

is really a step that happens chronologically.

[beaufortfrancois]

This still doesn't seem right, at least based on something that would make the examples work. Having "resolve p" as step 7 makes it sound like the promise doesn't get resolved until after the browser stops trying to detect NFC technology. I suspect the intent is to resolve the promise after "4. Add reader to the activated reader objects.", although it's possible that it could be right before (4) as well (which would be detectably different).

I should have resolved |p| earlier to make it clear indeed. I've opened #483 to fix it.

(I'd also note that (2) and (3) don't really seem like separate steps. But it also seems like, implicitly, between them, there's something like "wait for this request to ???", so that they could be combined as something like: " If this is the first listener being set up, then make a request to all NFC adapters to listen to NDEF messages. Wait for this request to ???. If the request fails, then the UA MAY reject p with a "NotSupportedError" DOMException and abort these steps." (Though this also seems to forbid a UA from trying again if it fails once, which might be undesirable.)

@zolkis @kenchris I'm not sure what "make a request to all NFC adapters to listen to NDEF messages." means? Is this a concept from the previous spec version? We're already checking if no underlying NFC Adapter, if a connection cannot be established, and if the UA is not allowed to access the underlying NFC Adapter (e.g. a user preference) earlier, so I'm not sure what this refers to?

Likewise, it's not clear to me that step 5, currently:

If the Document of the top-level browsing context is not visible (e.g. the user navigated to another page), then the registered activated reader objects still SHOULD continue to exist, but SHOULD become paused, i.e. the UA SHOULD NOT check and use them until the Document is visible again.

is really a step that happens chronologically.

As we already have https://w3c.github.io/web-nfc/#handling-visibility-change, we may want to remove step 5 here. What do you think @kenchris @zolkis?

[zolkis]

I'm not sure what "make a request to all NFC adapters to listen to NDEF messages." means?

If there are multiple NFC adapters (e.g. a built-in and a USB adapter), they are all used in parallel. There is no API needed to list them or select one. It does not matter which one makes the reading (the closest).

[zolkis]

As we already have https://w3c.github.io/web-nfc/#handling-visibility-change, we may want to remove step 5 here.

I agree.

[beaufortfrancois]

@dbaron I believe all your feedback has been addressed. See https://w3c.github.io/web-nfc/#the-scan-method

Return p and run the following steps in parallel:

1. If the obtain permission steps return false, then reject p with a "NotAllowedError" DOMException and abort these steps.
2. Add reader to the activated reader objects.
3. Resolve p.
4. Whenever the UA detects NFC technology, run the NFC reading algorithm.

[dbaron]

So currently the text of step 7 says:

Return p and run the following steps in parallel:

1. If the obtain permission steps return false, then reject p with a "NotAllowedError" DOMException and abort these steps.
2. If there is no underlying NFC Adapter, or if a connection cannot be established, then reject p with a "NotSupportedError" DOMException and return p.
3. If the UA is not allowed to access the underlying NFC Adapter (e.g. a user preference), then reject p with a "NotReadableError" DOMException and return p.
4. Add reader to the activated reader objects.
5. Resolve p.
6. Whenever the UA detects NFC technology, run the NFC reading algorithm.

This now seems pretty close, except that items (2) and (3) above should say "and abort these steps" instead of "and return p".

[beaufortfrancois]

@dbaron Thanks for raising the issue. I've fixed text.