1115465 - Add id attribute to NFCTag

Status: RESOLVED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Every NFC tag should have an unique identifier (UID), although for some technology the UID is generated randomly at discovery (for example, NFCID-1 of length 4 used by NFC_A), and for some technology the naming is different, like Felica it's called IDm.

To make it consistent in WebAPI, I am thinking about add an 'id' attribute into NFCTag.

Comment 1

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch.

Comment 2

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8541623 [details] [diff] [review]
Patch.

Review of attachment 8541623 [details] [diff] [review]:
-----------------------------------------------------------------

forgot updating minor version.

Comment 3

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch. v2.

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: nfcd Pull Request

Comment 5

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8541697 [details] [diff] [review]
Patch. v2.

Review of attachment 8541697 [details] [diff] [review]:
-----------------------------------------------------------------

Forward r? to smaug for WebIDL changes. (MozNFCTag.webidl, NfcOptions.webidl)

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8541697 [details] [diff] [review]
Patch. v2.

>+  int32_t idCount = aParcel.readInt32();
>+  aOptions.mTagId.AppendElements(
>+      static_cast<const uint8_t*>(aParcel.readInplace(idCount)), idCount);
Nit, 2 spaces for indentation

Comment 7

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch. v3.

I also noticed some places doesn't use 2 spaces, so changed them as well.

Comment 8

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

https://hg.mozilla.org/integration/b2g-inbound/rev/c776f112102f

Comment 9

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

https://github.com/mozilla-b2g/platform_system_nfcd/commit/22cd128c866dd895fd2e019918982b147503ac80

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c776f112102f

Comment 11

[Boris Zbarsky [:bzbarsky]]

The IDL added in this bug:

  [Pure, Constant] readonly attribute Uint8Array? id;

is nonsense.  Is it Pure or Constant?  Those are mutually exclusive things (and I wrote a path that fails codegen when both are specified, but landing that is blocked on the IDL this bug added).

Comment 12

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Sorry, should be [Constant]. I'll file another bug to fix this.

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Boris Zbarsky [:bz] from comment #11)
> The IDL added in this bug:
> 
>   [Pure, Constant] readonly attribute Uint8Array? id;
> 
> is nonsense.  Is it Pure or Constant? 
Bah, sorry. Constant it should be


> Those are mutually exclusive things
How so. Even the documentation for [Constant] says
"This extended attribute implies [Pure] as far as the JIT is concerned." 
So per that [Pure] in this case is just not needed, but shouldn't cause any harm.

Comment 14

[Boris Zbarsky [:bzbarsky]]

> So per that [Pure] in this case is just not needed, but shouldn't cause any harm.

That depends on how the codegen is structured.  It happens to be the case that we check for "Constant" before checking for "Pure", but if we checked in the opposite order you'd get different behavior.

"Constant" means not affecting other things and not depending on other things.  "Pure" means not affecting other things but depending on other things. When both are specified, do we depend on other things?

Anyway, the DependsOn/Affects stuff I was trying to land makes that all much clearer, and I wanted to make sure people didn't specify DependsOn in combination with Pure and such, precisely because the results end up ambiguous.

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Might be good if MDN documentation about [Constant] wouldn't say anything about 'implies [Pure]'.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Yes, once I land bug 1118978 I'll update the documentation to be clearer.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Documentation updated at https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings#Pure and https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings#Constant

Comment 18

[Olli Pettay [:smaug][[e-mail]]]

Thanks.