Closed Bug 1059082 Opened 10 years ago Closed 10 years ago

[NFC] Could not share the contact which initial imported via NFC

Categories

(Firefox OS Graveyard :: Gaia::Contacts, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.1+, b2g-v2.1 verified, b2g-v2.2 fixed)

VERIFIED FIXED
2.1 S5 (26sep)
blocking-b2g 2.1+
Tracking Status
b2g-v2.1 --- verified
b2g-v2.2 --- fixed

People

(Reporter: ashiue, Assigned: arcturus)

References

Details

(Whiteboard: [p=1], [2.0-flame-test-run-3] [priority][p=4])

Attachments

(2 files)

+++ This bug was initially created as a clone of Bug #1034405 +++ Gaia 6e804a42ab90f4251c7fe8c68731dc1c6abd8006 Gecko https://hg.mozilla.org/mozilla-central/rev/0753f7b93ab7 BuildID 20140826181551 Version 34.0a1 STR: 1. Phone A receive contact via NFC (Phone A stay at the received contact info page) 2. Tap 2 phones together 3. Phone A swipe shrinking UI Expected result: Shrinking UI show correctly and the contact can be shared Actual result: 1. If Phone A launch contact app before receive contact via NFC, screen hang. ( http://youtu.be/hHwLA9UQV2k) 2. If Phone A launch any sharable app (except contact) before receive contact via NFC, phone A would send the content of sharable app. 3. If Phone A launch any unsharable apps or stay at homescreen before receive contact via NFC, phone A has no shrinking UI
[Blocking Requested - why for this release]:
blocking-b2g: --- → 2.1?
QA Whiteboard: [COM=NFC]
IMHO, this shouldn't block a release, it's definitely a nice to have.
Assignee: nobody → francisco
Whiteboard: [p=1], [2.0-flame-test-run-3] [priority] → [p=1], [2.0-flame-test-run-3] [priority][p=4]
Target Milestone: --- → 2.1 S1 (1aug)
triage: strong partner request
blocking-b2g: 2.1? → 2.1+
It seems fixed in bug 1034405, but came out again?
I found this issue does not fix when I tried to verify bug 1034405.
Again, IMHO, I dont think this bug should block the release.
NFC is important so I would need this addressed so we have a good user experience - remains 2.1+
In case of the STR result in point 1 (contacts app was opened earlier) the new contact is displayed in the inline activity opened on the top of contacts app. When trying to share this I can see that the inline activity is closed and regular contacts app screen is visible. What is interesting, in NfcManager logs i can see that the activity failed (onerror handler was fired), but the "import" activity defined in communications has |"returnValue": true| in manifest file. The "onerror" handler for activity seems to fire once the Shrinking UI animation starts. Do we have some other activity fired as a results of "import"? Maybe we should consider introducing a new activity handler in manifest.webapp ("nfc-ndef-discovered"?), specifically for contact import via NFC, which would just fire the contact app instead of inline activity?
I think the target milestone is outdated. Can you update it? Thanks.
Flags: needinfo?(whuang)
Target Milestone: 2.1 S1 (1aug) → 2.1 S5 (26sep)
Did you find out who/what fires the onerror code path? Activities should only deliver one message at a time, and prompt the "activity chooser" if the filters match more than one. Activities currently are DOMRequests too, so an app can actually return a response. (In reply to Krzysztof Mioduszewski[:kmioduszewski][:tauzen] from comment #8) > What is interesting, in NfcManager logs i can see that the activity failed > (onerror handler was fired), but the "import" activity defined in > communications has |"returnValue": true| in manifest file. The "onerror" > handler for activity seems to fire once the Shrinking UI animation starts.
Flags: needinfo?(whuang) → needinfo?(kmioduszewski)
I've been trying to dive into this issue, but when I try with 2 flames, one in 2.0 nightly and the other with current master, trying to send the contact from 2.0 to 2.2, I get the following error in the 2.2 phone: E/GeckoConsole( 1633): Content JS LOG at app://system.gaiamobile.org/js/nfc_manager.js:545 in nm_logVisibly: [NfcManager]: CheckP2PRegistration failed Shrinking ui appears in the first phone (2.0), but got that error on the receiver phone (2.2) Any idea of this, should I try with 2 phones on 2.2 then?
Flags: needinfo?(ashiue)
|CheckP2PRegistration failed| means that 2.2 flame detected the other device, but the current foreground app didn't register onpeerready handler, so does not want to share anything to other device. I might consider changing this message to something less disturbing. Still, if you did try to share from 2.0 (by doing a swipe) the 2.2 flame should receive it normally. Do you have problems with receiving the shared contact at all on 2.2? I'm just doing a fresh build right now, so I can try to answer the question from comment 10.
I've followed Garners advice and found out that the activity was actually canceled by the Contacts app itself as a result of 'visibilitychange' event basically triggered by the ShrinkingUI here [1]. I've prepared a quick fix [2] for this, after applying it I cannot replicate the error anymore and sharing works as expected. Francisco, I'm not familiar with the Contacts app, so I'm not sure if this won't cause errors in different scenarios. If you I think this solution is ok, I can send a pull request. If this still needs some work, please feel free to use it ;) [1] - https://github.com/mozilla-b2g/gaia/blob/7549939141343e23792e0a69e33225be340e9fb1/apps/system/js/shrinking_ui.js#L198 [2] - https://github.com/tauzen/gaia/commit/bb15a8102bb9425710d8e6201c541010a5d9a8ec
Flags: needinfo?(kmioduszewski) → needinfo?(francisco)
Attached file Pointer to PR 24059 (deleted) —
Removing the code to abort any activity if we are being hidden. We used this trick to avoid some problems with keyboard that won't happen any more, so bug 1028502 will be fix as well with same solution.
Attachment #8489441 - Flags: review?(jmcf)
Flags: needinfo?(francisco)
I've tested Francisco's fix in the sharing scenarios using flame 2.2 and Android. It works fine, I cannot replicate the original bug.
Comment on attachment 8489441 [details] Pointer to PR 24059 thanks Francisco
Attachment #8489441 - Flags: review?(jmcf) → review+
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment on attachment 8489441 [details] Pointer to PR 24059 [Approval Request Comment] [Bug caused by] (feature/regressing bug #): It's not a regression but a malfunction we have since we added the NFC functionlaity [User impact] if declined: User's wont be able to share repeately a contact. Major NFC feature request from partners [Testing completed]: Smoke test done. [Risk to taking this patch] (and alternatives if risky): Pretty low, in the patch we are just removing code to abort activities when contacts go to the background. [String changes made]:
Attachment #8489441 - Flags: approval-gaia-v2.1?(fabrice)
Attachment #8489441 - Flags: approval-gaia-v2.1?(fabrice) → approval-gaia-v2.1+
Verified on [master] Gaia-Rev 3c898380b47f298cd3b7a0dacb3a6529e94322d4 Gecko-Rev https://hg.mozilla.org/mozilla-central/rev/4cdc4b9e5832 Build-ID 20140922184244 Version 35.0a1 [2.1] Gaia-Rev 3742913e11f69e789dcb0aa0dedf2e5572da0129 Gecko-Rev https://hg.mozilla.org/releases/mozilla-aurora/rev/df42b05782aa Build-ID 20140922185144 Version 34.0a2
Status: RESOLVED → VERIFIED
Flags: needinfo?(ashiue)
Attached video video of issue verify (deleted) —
This issue has been successfully verified on Flame 2.1 Steps: 1. Device A Launch Contacts app. 2. Receive a contacts from device B via NFC. 3. Shara the above contacts from device A to device B via NFC. **It can be shara successfully and tap home key can back to home. See attachment: verify_video.MP4 Reproducing rate: 0/5 Flame 2.1 versions: Gaia-Rev 1b231b87aad384842dfc79614b2a9ca68a4b4ff3 Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/95fbd7635152 Build-ID 20141119001205 Version 34.0 Device-Name flame FW-Release 4.4.2 FW-Incremental eng.cltbld.20141119.035246 FW-Date Wed Nov 19 03:52:56 EST 2014 Bootloader L1TC00011880
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: