If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[NFC] Remove mozNfc checks from NfcManager

RESOLVED FIXED in 2.2 S9 (3apr)

Status

Firefox OS
NFC
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: tauzen, Assigned: tauzen)

Tracking

(Blocks: 1 bug)

unspecified
2.2 S9 (3apr)
x86
Mac OS X

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
NfcManager is instantiated (via NfcCore) only when mozNfc API is available [0]. All the |if(mozNfc)| checks should be removed. 

[0] - https://github.com/mozilla-b2g/gaia/blob/d36676d0a02eb13f5eb358d5f2413a9bba103c40/apps/system/js/core.js#L39-L57
(Assignee)

Updated

3 years ago
Blocks: 933640
(Assignee)

Updated

3 years ago
Assignee: nobody → kmioduszewski

Comment 1

3 years ago
Created attachment 8581823 [details] [review]
[gaia] tauzen:Bug1146400_no_nfcdom_check > mozilla-b2g:master
(Assignee)

Comment 2

3 years ago
Comment on attachment 8581823 [details] [review]
[gaia] tauzen:Bug1146400_no_nfcdom_check > mozilla-b2g:master

Hi Greg, this is a small clean up patch. I've removed the unnecessary |mozNfc| checks, changed |dispatchEvent| to |this.publish| (from BaseModule). Additionally I've added comments which I forgot to add in Bug 1141490. Could you review this?
Attachment #8581823 - Flags: review?(gweng)
Comment on attachment 8581823 [details] [review]
[gaia] tauzen:Bug1146400_no_nfcdom_check > mozilla-b2g:master

OK I've checked the patch it's good if the instance existing is guaranteed by the new system architecture.
Attachment #8581823 - Flags: review?(gweng) → review+
(Assignee)

Comment 4

3 years ago
Yes, exactly. NfcManager is instantiated only if mozNfc API is available. This was actually pointed out by Alive [0]. Thank you very much for the review.

[0] - https://github.com/mozilla-b2g/gaia/pull/28962#discussion_r26729256
Keywords: checkin-needed

Updated

3 years ago
Keywords: checkin-needed

Comment 5

3 years ago
http://docs.taskcluster.net/tools/task-graph-inspector/#z0kVDb9qSbGDHgz5tGPzmw

The pull request failed to pass integration tests. It could not be landed, please try again.
(Assignee)

Comment 6

3 years ago
All tests on github are green [0], so let's try this once more.

[0] - https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=4f78f12181b6fbe0e2490d865c7b1a0cdbbfa6c5
Keywords: checkin-needed

Updated

3 years ago
Keywords: checkin-needed

Comment 7

3 years ago
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/d5cb256ca570c3ea206923f2f0e9c98185744d45

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Updated

3 years ago
Target Milestone: --- → 2.2 S9 (3apr)
You need to log in before you can comment on or make changes to this bug.