Closed Bug 860910 Opened 7 years ago Closed 6 years ago

[B2G][Gaia][NFC] NFC Manager

Categories

(Firefox OS Graveyard :: Gaia, defect)

x86
macOS
defect
Not set

Tracking

(blocking-b2g:1.3+)

RESOLVED FIXED
1.3 Sprint 5 - 11/22
blocking-b2g 1.3+

People

(Reporter: qdot, Assigned: dgarnerlee)

References

Details

(Whiteboard: [FT:RIL])

Attachments

(1 file, 16 obsolete files)

46 bytes, text/x-github-pull-request
alive
: review+
Details | Review
Integration of gaia demo app and system call for activities for NFC
Demo code for NFC. Read/write/P2P NDEF tags, and manifest activities support.
Modifies System app to redirect messages to registered activities in application manifests.
Proposed, and starting point, changes to other certified apps to allow basic NFC activity support for dialer, contacts, and browser URL.
Attachment #743251 - Flags: review?(kyle)
Attachment #743255 - Flags: review?(kyle)
Attachment #743257 - Flags: review?(kyle)
I'm not qualified to review any of this. We need to find someone on the gaia team to take a look at it, and probably after bug 860906 lands since there may be changes in that which change this.
Attachment #743251 - Flags: review?(kyle)
Attachment #743255 - Flags: review?(kyle)
Attachment #743257 - Flags: review?(kyle)
Update for dom name change: MozNdefRecord --> NdefRecord
Attachment #743251 - Attachment is obsolete: true
Update for NfcEvent field name change to "message".
Attachment #743969 - Attachment is obsolete: true
Rebased on top of new mozillaorg master.
Attachment #768662 - Attachment is obsolete: true
Attachment #743253 - Attachment is obsolete: true
Attachment #743255 - Attachment is obsolete: true
Just had a very quick skim on this - the demo makes heavy use of web activities which worries me (noting that any web content, including web pages can initiate web activities). Maybe this is a just a demo though and we don't care so much about security though ? I was expecting that eventually there would be a separate NFC app that has the NFC permission and listens to the appropriate system messages - is the the plan, or are we planing to keep NFC in the system app?
Flags: needinfo?(dgarnerlee)
This is a different logic layer than covered by a security review of "WebNFC" (bug 674741, bug 749325) and needs a separate sec-review (probably smallish).
Flags: sec-review?
I changed the secreview(bug 749325) to block the parent of 674741, which is actually the meta bug that includes this bug and many other NFC bugs which all need at least some review I think. So removing this flag since it will be covered by the existing secreview.
Flags: sec-review?
Hi,

I note Contacts should be one of the user stories that should be suppported. In the System App/nfc.js patch code, I have this:

+  // FIXME, incomplete mapping/formatting. App (or full featured vcard parser
+  // library should parse the full ndef vcard.
+  // VCARD 2.1:
+  function handleVCardRecord(record) {

This suggests some bit of architecure: Will there be a generally available VCARD parser library, service, xpcom component, etc. between contacts, and the system nfc manager? It would seem there should be a already vetted VCARD parser already. If so, we can get rid of the FIXME with common code.
Flags: needinfo?(dgarnerlee) → needinfo?
(In reply to Paul Theriault [:pauljt] from comment #12)
> Just had a very quick skim on this - the demo makes heavy use of web
> activities which worries me (noting that any web content, including web
> pages can initiate web activities). Maybe this is a just a demo though and
> we don't care so much about security though ? I was expecting that
> eventually there would be a separate NFC app that has the NFC permission and
> listens to the appropriate system messages - is the the plan, or are we
> planing to keep NFC in the system app?

nfc-demo is currently sort of intended to be a catch all for registered nfc-ndef-discovered messages, it probably shouldn't ship, at least, not as nfc-demo. It's currently the only app that can write tags however, so it's not quite purely a demo app anymore. The system app can deliver VCARDs and URLs to Contacts and Browser. If more than one is registered, then the user needs to select one.

From the security perspective, app/system/nfc.js (or soon: nfc_manager.js), will gain "nfc-manager" permissions to process techdiscovery/techlost messages from the NFC deamon, and we'll remove techdiscovery/lost from the DOM eventually.

The current plans are: From there, it routes the message via web activities to the apps (popping up a message to choose the app). From that perspecitve, it's just launching any application registered for that type, similar to how Android Intents work. The selected app then can use DOM calls (connect/disconnect/read/write/push/details). 

Applications themselves will require a "connect(mozNfc.NDEF)", to ensure a 1 to 1 connection to hardware (at least, until you switch foreground NFC apps. That needs some discussion from both UX and security: how long is an app allowed to stay connected, without disconnection, locking a resource). Currently, certified apps are needed, but that will change.
Flags: needinfo?
> The current plans are: From there, it routes the message via web activities
> to the apps (popping up a message to choose the app). From that perspecitve,
> it's just launching any application registered for that type, similar to how
> Android Intents work. The selected app then can use DOM calls
> (connect/disconnect/read/write/push/details). 

This is what sounds dangerous to me, and also what I am really having trouble following. Unlike Android Intents, Web Activities can not be explicitly routed to a given application - the user chooses. Furthermore, if there is only one suitable application registered to handle a specific web activity action/filter combination, it will be chosen without prompting the user at all.

Maybe this has been thought of, but what prevents the following threats:

- A web page starting one of the web activities sent by the system app? (is it up to the receiving app to verify the origin of the web activity initiator?)

- A web app registering to handle one of the activities used (nfc-ndef-discovered, nfc-write-request-status-disabled, nfc-ndefdisconnected-disabled) since any web app can register to handle any web activity

Does this design take into account these threats?
Note, since you asked in email - an app can currently only register themselves as handling a specific activity by declaring them in the manifest. Handling web activities in regular web pages is not yet supported, although this was discussed as a possibility. See https://developer.mozilla.org/en-US/docs/WebAPI/Web_Activities#Register_an_activity and also bug 775181. 

The main concern I have from a security perspective is that any hosted app could handle an activity (i.e. self installed, not reviewed by marketplace).

I don't you can register wildcard (*) for the activity name, but filters support wildcards I think.
Blocks: 894689, 894323
No longer blocks: b2g-nfc
Component: General → Gaia
Hi, I do have some concern about launching app via activity.
In the future we should let each app be able to use NFC,
but we definitely don't want to launch each app by activity hardcoded in system app every we have a new app.

I expect that NFC filter is done in gecko layer,
and use system message with a specific parameter like web activity to tell window manager to open the app.

With current implementation it's impossible to do https://bugzilla.mozilla.org/show_bug.cgi?id=920882
Hi Garner, any feedback?
Flags: needinfo?(dgarnerlee)
I discussed this with Fabrice in mozilla summit.
We both agree we should move the logic into gecko but it's uncertain how to do that now.

Fabrice said he would cowork with you guys to resolve gecko part.
We actually have a meeting with Fabrice on Activities today on limitations and solutions, will update.
Ahead of the meeting, the system app (via system/js/nfc_manager.js) routes the messages to different apps based on filtering. Is security inherently weaker in the system app (gaia), than through the parent gecko/dom app process?
Flags: needinfo?(dgarnerlee)
What matters is not security issue.
blocking-b2g: --- → 1.3+
Attachment #789830 - Attachment is obsolete: true
Attachment #820718 - Attachment description: NFC Demo and manifests (no JQuery) → Patch (v5) NFC Demo and manifestsno JQuery)
Attachment #820718 - Attachment description: Patch (v5) NFC Demo and manifestsno JQuery) → Patch (v5) NFC Demo and manifests (no JQuery)
NFC Manager routes messages from the parent gecko/gonk/nfcd processes.
Attachment #789832 - Attachment is obsolete: true
Attachment #820721 - Attachment description: NFC System app integration: activities and messages, airplane mode. → Patch (v3) NFC System app integration: activities and messages, airplane mode.
We will use this bug for NFC manager in Gaia.
Summary: B2G Gaia Integration for NFC → [B2G][Gaia][NFC] NFC Manager for NFC
Merged 11/4 code. Code and manifest changes. Initail reviewer: Assigning to Alive.
Attachment #820721 - Attachment is obsolete: true
Attachment #827569 - Flags: review?(alive)
Garner, should we remove those demonstrating patches from this bug?
Flags: needinfo?(dgarnerlee)
Attachment #789831 - Attachment is obsolete: true
Attachment #789835 - Attachment is obsolete: true
Attachment #820718 - Attachment is obsolete: true
Demo/Example code removed.
Flags: needinfo?(dgarnerlee)
Comment on attachment 827569 [details] [diff] [review]
Patch (v4) NFC System app integration: activities and messages,  airplane mode.

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

Sorry, the readability, quality and testability of nfc manager is really low.
Please rewrite it into object literal instead of module pattern.

See other module in system app for hint.

I don't really know what you do to mediadb, pass review to David.

::: apps/system/js/nfc_manager.js
@@ +6,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this file,
> + * You can obtain one at http://mozilla.org/MPL/2.0/.
> + */
> +
> +/* Copyright © 2013, Deutsche Telekom, Inc. */

Please remove this line.

@@ +11,5 @@
> +
> +'use strict';
> +
> +(function() {
> +  var DEBUG = true;

nit: Do you mean to enable DEBUG?

@@ +61,5 @@
> +    RFUAction: 0x03,  // Reserved from 0x03 to 0xFF
> +
> +    uris: new Array(),
> +
> +    init: function() {

give function a name

@@ +137,5 @@
> +    if (acceptEvents) {
> +      powerLevel = NFC_POWER_LEVEL_ENABLED;
> +    }
> +    var request = navigator.mozSettings.createLock().set({
> +      'nfc.powerlevel': powerLevel

What's this for?

@@ +154,5 @@
> +  function handleEvent(evt) {
> +    debug('XXXXXXXXX Handle Event evt.type' + evt.type);
> +    switch (evt.type) {
> +      case 'screenchange':
> +        screenEnabled = evt.detail.screenEnabled;

Don't need to record this.

@@ +271,5 @@
> +   * Tags, and fallback tag handling.
> +   */
> +  function acceptNfcEvents() {
> +    // Policy:
> +    if (screenEnabled && !screenLocked) {

ScreenManager.screenEnabled && !LockScreen.locked

@@ +438,5 @@
> +
> +  function handleTechLost(command) {
> +    debug('Technology Lost: ' + JSON.stringify(command));
> +    var a = new MozActivity({
> +      name: 'nfc-tech-lost',

Why do we invoke an activity here????
Attachment #827569 - Flags: review?(dflanagan)
Attachment #827569 - Flags: review?(alive)
Attachment #827569 - Flags: review-
Hi, do we have a clear view on the target milestone on this bug now? Thanks.
Comment on attachment 827569 [details] [diff] [review]
Patch (v4) NFC System app integration: activities and messages,  airplane mode.

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

::: shared/js/mediadb.js
@@ +1309,5 @@
> +    };
> +
> +    enumerateNextStorage();
> +    return cursor;
> +  }

This code doesn't seem to have anything to do with the NFC patch.  This is old code that has been removed from mediadb.js, I think. It should not be here.

I suspect that this is unintentional and the patch just needs to be rebased.
Attachment #827569 - Flags: review?(dflanagan) → review-
That appears to be the case. Yes, I'll rebase. I wanted to get the code up somewhat early for people to comment on first, so it was more or less a first pass.

The NFC DOM itself has been modified quite a bit, with nfc_manager along for the ride, so I'll clean it up.

(In reply to David Flanagan [:djf] from comment #33)
> Comment on attachment 827569 [details] [diff] [review]
> Patch (v4) NFC System app integration: activities and messages,  airplane
> mode.
> 
> Review of attachment 827569 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: shared/js/mediadb.js
> @@ +1309,5 @@
> > +    };
> > +
> > +    enumerateNextStorage();
> > +    return cursor;
> > +  }
> 
> This code doesn't seem to have anything to do with the NFC patch.  This is
> old code that has been removed from mediadb.js, I think. It should not be
> here.
> 
> I suspect that this is unintentional and the patch just needs to be rebased.
It seems better to finish this bug before sprint5. If you think it isn't reasonable, please update target milestone.
Target Milestone: --- → 1.3 Sprint 5 - 11/22
Duplicate of this bug: 933591
WIP. Implmented most changes. Mainly convert nfc_manager.js to class based impl.

Added apache header. TechLost is currently used by nfc-demo, though we can still decide to remove it. Powerlevel is still needed while new settings values are added to properly manage the power on nfcd (User UI switch changes preference, and nfc manager goes and makes it effective on NFCD).
Attachment #832008 - Flags: feedback?(alive)
Comment on attachment 827569 [details] [diff] [review]
Patch (v4) NFC System app integration: activities and messages,  airplane mode.

Obsoleting. Patch v4 is still usable in case someone is using it. The new patch requires nfc_manager to use Uint8Array, and a matching NFC DOM implementation.
Attachment #827569 - Attachment is obsolete: true
Comment on attachment 832008 [details] [diff] [review]
0001-Patch-v5-NFC-System-app-integration-activies-and-mes.patch

Changing feedback to review request, as per email.
Attachment #832008 - Flags: review?(alive)
Blocks: 933585
Comment on attachment 832008 [details] [diff] [review]
0001-Patch-v5-NFC-System-app-integration-activies-and-mes.patch

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

Basically not r- but pay attention to the comments.

BTW we still have something to do, please file followup bugs.
For example, it doesn't make sense to use settings API to tell gecko the so-called power level.

::: apps/system/js/nfc_manager.js
@@ +157,5 @@
> +    this._debug('Initializing NFC Message');
> +    var self = this;
> +    window.navigator.mozSetMessageHandler(
> +      'nfc-manager-tech-discovered',
> +      function callTechnologyDiscovered(message) {

this.handleTechnologyDiscovered.bind(this)

@@ +162,5 @@
> +         self.handleTechnologyDiscovered(message);
> +      });
> +    window.navigator.mozSetMessageHandler(
> +      'nfc-manager-tech-lost',
> +      function callHandleTechnologyLost(message) {

this.self.handleTechLost.bind(this)

@@ +551,5 @@
> +         / "NOTE" / "PRODID" / "REV" / "SOUND" / "UID" / "CLIENTPIDMAP"
> +         / "URL" / "KEY" / "FBURL" / "CALADRURI" / "CALURI" / "XML"
> +         / iana-token / x-name
> +     */
> +    var fn = findKey('FN:', payload);

I think think this method call works.

@@ +646,5 @@
> +
> +  handleSmartPosterAction: function nm_nm_handleSmartPosterAction(record) {
> +    // The recommended action has an application specific meaning:
> +    var smartaction = record.payload[0];
> +    var activityText = {

Why not put all these activityText inside an object and access it via specific key?
Attachment #832008 - Flags: review?(alive)
Attachment #832008 - Flags: review+
Attachment #832008 - Flags: feedback?(alive)
Duplicate of this bug: 903261
Summary: [B2G][Gaia][NFC] NFC Manager for NFC → [B2G][Gaia][NFC] NFC Manager
I'll make the fixes and updates.

Concerning powerlevel, see Bug 933184 (we'll fix it there):
There's current plans to consider the W3C version of the DOM (and the related security questions), or just send an event down to Nfc.js in system/gonk. That particular fix is needed to fix a race condition on managing startup and power states on the nfc hardware.
Comment on attachment 832008 [details] [diff] [review]
0001-Patch-v5-NFC-System-app-integration-activies-and-mes.patch

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

Changed my mind since I think I need to be careful to avoid we have something nobody really understand "why does this work" or "why do we have something doesn't really work but in the code".

::: apps/system/js/nfc_manager.js
@@ +186,5 @@
> +    if (acceptEvents) {
> +      powerLevel = this.NFC_POWER_LEVEL_ENABLED;
> +    }
> +    var request = navigator.mozSettings.createLock().set({
> +      'nfc.powerlevel': powerLevel

Send a mozContentEvent to shell.js and notify nfcd to stop instead of polluting settings API.

@@ +208,5 @@
> +  },
> +
> +  // An NDEF Message is an array of one or more NDEF records.
> +  handleNdefMessages: function nm_handleNdefMessages(ndefmessages) {
> +    var action = new Array();

actions

@@ +297,5 @@
> +
> +    this._debug('handleNdefDiscovered: ' + JSON.stringify(ndefMsg));
> +    var records = ndefMsg;
> +    this._debug('handleNdefDiscovered: ' + JSON.stringify(records));
> +    var action = this.handleNdefMessages(records);

actions

@@ +315,5 @@
> +  // TODO:
> +  handleNdefFormattableDiscovered:
> +    function nm_handleNdefFormattableDiscovered(tech, session) {
> +
> +    return this.handleNdefDiscoveredNotification(tech, session);

one more indent

@@ +316,5 @@
> +  handleNdefFormattableDiscovered:
> +    function nm_handleNdefFormattableDiscovered(tech, session) {
> +
> +    return this.handleNdefDiscoveredNotification(tech, session);
> +  },

one more indent

```js
var A = {
  tooooolongfunnnnnctionnnnnname:
    function justootoototlongfunctionname() {
      var a = 1;
    }
};

```

@@ +331,5 @@
> +
> +    // Check for tech types:
> +    this._debug('command.tech: ' + command.tech);
> +    var handled = false;
> +    var techs = command.tech;

Is this an array or what?

@@ +334,5 @@
> +    var handled = false;
> +    var techs = command.tech;
> +    if (command.ndef.length) {
> +      // Pick the first NDEF message for now.
> +      var ndefMsg = command.ndef[0];

What? the scope of this variable is only in the braces.
Also what about command.ndef[1+] ?

@@ +336,5 @@
> +    if (command.ndef.length) {
> +      // Pick the first NDEF message for now.
> +      var ndefMsg = command.ndef[0];
> +    } else {
> +      var ndefMsg = [];

scope problem

@@ +343,5 @@
> +    // Force Tech Priority:
> +    var pri = ['P2P', 'NDEF', 'NDEF_FORMATTABLE', 'NFC_A', 'MIFARE_ULTRALIGHT'];
> +    for (var ti = 0; ti < pri.length; ti++) {
> +      this._debug('Going through NFC Technologies: ' + ti);
> +      var i = techs.indexOf(pri[ti]);

I totally don't understand the meaning of this statement.

@@ +344,5 @@
> +    var pri = ['P2P', 'NDEF', 'NDEF_FORMATTABLE', 'NFC_A', 'MIFARE_ULTRALIGHT'];
> +    for (var ti = 0; ti < pri.length; ti++) {
> +      this._debug('Going through NFC Technologies: ' + ti);
> +      var i = techs.indexOf(pri[ti]);
> +      if (i != -1) {

if (i < 0) {
  return;
}

@@ +346,5 @@
> +      this._debug('Going through NFC Technologies: ' + ti);
> +      var i = techs.indexOf(pri[ti]);
> +      if (i != -1) {
> +        if (techs[i] == 'P2P') {
> +          if (!ndefMsg) {

You don't have ndefMsg here!

@@ +359,5 @@
> +            //       system application notifies gecko of the top most window.
> +
> +            // Notify gecko of User's acknowledgement.
> +            var nfcdom = window.navigator.mozNfc;
> +            nfcdom.setPeerWindow(window.top);

I don't see this in API doc. I do think you should pass window.top

@@ +384,5 @@
> +        }
> +      }
> +    }
> +
> +    if (handled == false) {

===

@@ +551,5 @@
> +         / "NOTE" / "PRODID" / "REV" / "SOUND" / "UID" / "CLIENTPIDMAP"
> +         / "URL" / "KEY" / "FBURL" / "CALADRURI" / "CALURI" / "XML"
> +         / iana-token / x-name
> +     */
> +    var fn = findKey('FN:', payload);

I don't *

@@ +557,5 @@
> +    var p = fn.indexOf(' ');
> +    var first = null;
> +    var last = null;
> +    if (p == 0) {
> +      var first = fn.substring(0, fn.indexOf(' '));

scope error!

@@ +559,5 @@
> +    var last = null;
> +    if (p == 0) {
> +      var first = fn.substring(0, fn.indexOf(' '));
> +    } else {
> +      var first = fn.substring(0, fn.indexOf(' '));

scope error!

@@ +560,5 @@
> +    if (p == 0) {
> +      var first = fn.substring(0, fn.indexOf(' '));
> +    } else {
> +      var first = fn.substring(0, fn.indexOf(' '));
> +      var last = fn.substring(fn.indexOf(' ') + 1);

scope error!

@@ +572,5 @@
> +    var note = findKey('NOTE:', payload);
> +    var address = findKey('ADR;HOME:', payload);
> +    var company = findKey('ADR;WORK:', payload);
> +
> +    // Add fields:

Why not use an objec/array to iterate this.findKey and doing assign instead of all if below
Attachment #832008 - Flags: review+
Hi Alive,

I'm working on this, but do you prefer to post a link to the pull request to github, or do the review here (it seems to go both ways in Gaia?).

To answer the question about tech priority:
An NFC tag can present itself as multiple different types. For example, the tags we have "NFC MiFare Ultralight" technology tags, with 142 KB free, can be read as a NFC_A tag, a MiFare tag (NXP proprietary), or a standard NDEF tag. We just choose P2P (for sharing), and NDEF as the supported technologies for now.

The tech list, is currently presented as "tech" from the NFC Chrome Worker you pointed out (We should probably fix that next time the NFC Chrome Worker is patched...).
Flags: needinfo?(alive)
(In reply to Garner Lee from comment #44)
> Hi Alive,
> 
> I'm working on this, but do you prefer to post a link to the pull request to
> github, or do the review here (it seems to go both ways in Gaia?).

Please do pull request. THX.

> 
> To answer the question about tech priority:
> An NFC tag can present itself as multiple different types. For example, the
> tags we have "NFC MiFare Ultralight" technology tags, with 142 KB free, can
> be read as a NFC_A tag, a MiFare tag (NXP proprietary), or a standard NDEF
> tag. We just choose P2P (for sharing), and NDEF as the supported
> technologies for now.
> 
> The tech list, is currently presented as "tech" from the NFC Chrome Worker
> you pointed out (We should probably fix that next time the NFC Chrome Worker
> is patched...).

This naming is really bad for understanding the code...
Flags: needinfo?(alive)
Attachment #832008 - Attachment is obsolete: true
Attachment #833399 - Attachment description: NFC System app integration: activies and messages, and airplane mode. → NFC System app integration: activies and messages, and airplane mode (v6)
Attachment #833399 - Flags: review?(alive)
Comment on attachment 833399 [details] [review]
NFC System app integration: activies and messages, and airplane mode (v6)

1. Remove power level settings or patch gecko before landing this. NO settings API pollution.
2. Please look and read all your handleXXXXX functions again:
   (1) Some of them handle something.
   (2) Some of them format parameter object.
   (3) Some of them do nothing but call another function.
3. DOM Request are asynchronous.
Attachment #833399 - Flags: review?(alive)
Blocks: 903305
Flip dependency: this bug absolutely depends on Bug 933585 on the NFC DOM to operate, which depends on Bug 933497
No longer blocks: 933585
Depends on: 933585
Depends on: 933184
Switch dependency to more active bug on the same topic: Bug 934835
Depends on: 934835
No longer depends on: 933184
Attached file NFC Manager Implemenation (obsolete) —
This patch includes power state changes via messages that have just been pushed to Gecko and Gaia.
Attachment #833399 - Attachment is obsolete: true
Attachment #8335027 - Flags: review?(alive)
NFC UI/UX Question (There's no user story covering this):

Some of this can be partially initiated by NFC Manager:

Should the UI do anything when the tag/P2P device is becomes available (handleTechDiscovered)?
When the tag/P2P device becomes unavailable (handleTechLost)?

A sound or vibrate makes some sense when the tag is found or lost (Accessability), etc.)

// Play sound, or do something tactile:
window.navigator.vibrate([200]);

Also, techLost sort of makes no sense to apps. There's nothing for the app to do (except update a UI).
Just attaching anyone for UX comment for now...
Flags: needinfo?
Flags: needinfo?(kchang)
I've updated the NFC Manager on the same pull branch request (for review). I didn't wait for the P2P updates.
(In reply to Garner Lee from comment #51)
> NFC UI/UX Question (There's no user story covering this):
> 
> Some of this can be partially initiated by NFC Manager:
> 
> Should the UI do anything when the tag/P2P device is becomes available
> (handleTechDiscovered)?
> When the tag/P2P device becomes unavailable (handleTechLost)?
> 
> A sound or vibrate makes some sense when the tag is found or lost
> (Accessability), etc.)
> 
> // Play sound, or do something tactile:
> window.navigator.vibrate([200]);
> 
> Also, techLost sort of makes no sense to apps. There's nothing for the app
> to do (except update a UI).

Let's add this later....:).
Flags: needinfo?(kchang)
Flags: needinfo?
Comment on attachment 8335027 [details] [review]
NFC Manager Implemenation

Basically r+, please track the followups.

Thanks!
Attachment #8335027 - Flags: review?(alive) → review+
(In reply to Garner Lee from comment #53)
> I've updated the NFC Manager on the same pull branch request (for review). I
> didn't wait for the P2P updates.

Should we create a follow up bug?
Blocks: 941765
Pull request updated with review comments. Concerning gecko knowledge of the security lock screen state, I've only currently found mozPower to know about WakeLock, but not security lock. If that's not true, I'll open a new bug for followup. If all checks fine, let me know, and I'll add r=alive.
No longer blocks: 894689
Depends on: 894689
This new pull is a cherry-pick commit top of a new and current merge-able commit base.
Attachment #8335027 - Attachment is obsolete: true
Comment on attachment 8337181 [details] [review]
NFC Manager implementation.

Same commit change as before, just rebased.
Attachment #8337181 - Flags: review?(alive)
Comment on attachment 8337181 [details] [review]
NFC Manager implementation.

Still one test failed.
https://travis-ci.org/mozilla-b2g/gaia/jobs/14402274

Please try again.
Attachment #8337181 - Flags: review?(alive) → review+
Green with re-push!
https://travis-ci.org/mozilla-b2g/gaia/builds/14515364

What is going on with the persona test case causing it to fail?
Don't know. BTW if you need to merge, needinfo me.
Check-in needed (in Gaia).
Flags: needinfo?(alive)
Merged
https://github.com/mozilla-b2g/gaia/commit/5376482f9e7f1724195001146eb132f98dd53209
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(alive)
Resolution: --- → FIXED
Blocks: 943179
Blocks: 933136
Blocks: 943691
Whiteboard: [FT:RIL]
You need to log in before you can comment on or make changes to this bug.