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)
Tracking
(blocking-b2g:2.1+, 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)
(deleted),
text/x-github-pull-request
|
jmcf
:
review+
fabrice
:
approval-gaia-v2.1+
|
Details |
(deleted),
video/mp4
|
Details |
+++ 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
Reporter | ||
Comment 1•10 years ago
|
||
[Blocking Requested - why for this release]:
blocking-b2g: --- → 2.1?
QA Whiteboard: [COM=NFC]
Assignee | ||
Comment 2•10 years ago
|
||
IMHO, this shouldn't block a release, it's definitely a nice to have.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → francisco
Whiteboard: [p=1], [2.0-flame-test-run-3] [priority] → [p=1], [2.0-flame-test-run-3] [priority][p=4]
Assignee | ||
Updated•10 years ago
|
Target Milestone: --- → 2.1 S1 (1aug)
Comment 4•10 years ago
|
||
It seems fixed in bug 1034405, but came out again?
Reporter | ||
Comment 5•10 years ago
|
||
I found this issue does not fix when I tried to verify bug 1034405.
Assignee | ||
Comment 6•10 years ago
|
||
Again, IMHO, I dont think this bug should block the release.
Comment 7•10 years ago
|
||
NFC is important so I would need this addressed so we have a good user experience - remains 2.1+
Comment 8•10 years ago
|
||
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?
Comment 9•10 years ago
|
||
I think the target milestone is outdated. Can you update it? Thanks.
Flags: needinfo?(whuang)
Assignee | ||
Updated•10 years ago
|
Target Milestone: 2.1 S1 (1aug) → 2.1 S5 (26sep)
Comment 10•10 years ago
|
||
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.
Updated•10 years ago
|
Flags: needinfo?(whuang) → needinfo?(kmioduszewski)
Assignee | ||
Comment 11•10 years ago
|
||
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)
Comment 12•10 years ago
|
||
|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.
Comment 13•10 years ago
|
||
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)
Assignee | ||
Comment 14•10 years ago
|
||
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)
Comment 15•10 years ago
|
||
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 16•10 years ago
|
||
Comment on attachment 8489441 [details]
Pointer to PR 24059
thanks Francisco
Attachment #8489441 -
Flags: review?(jmcf) → review+
Assignee | ||
Comment 17•10 years ago
|
||
Landed in master:
https://github.com/mozilla-b2g/gaia/commit/b6a0b13dc9863d2c2c5e756d5a863d5d3b345b89
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 18•10 years ago
|
||
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)
Updated•10 years ago
|
status-b2g-v2.1:
--- → affected
status-b2g-v2.2:
--- → fixed
Updated•10 years ago
|
Attachment #8489441 -
Flags: approval-gaia-v2.1?(fabrice) → approval-gaia-v2.1+
Comment 19•10 years ago
|
||
Reporter | ||
Comment 20•10 years ago
|
||
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)
Comment 21•10 years ago
|
||
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.
Description
•