Closed
Bug 1034405
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
(tracking-b2g:backlog, b2g-v2.0 affected)
Tracking | Status | |
---|---|---|
b2g-v2.0 | --- | affected |
People
(Reporter: ashiue, Assigned: mbudzynski)
References
Details
(Whiteboard: [p=1], [2.0-flame-test-run-3] [priority])
Attachments
(2 files, 1 obsolete file)
Gaia 6b6b7d7fe829ebea85b01aa4ed44cf5ada366bbe
Gecko https://hg.mozilla.org/releases/mozilla-aurora/rev/9457a67841b3
BuildID 20140703160208
Version 32.0a2
STR:
1. Phone A receive contact via NFC (Phone A stay at the contact info page)
2. Tap 2 phones together
Expected result:
Shrinking UI show correctly and the contact can be shared
Actual result:
No shrinking UI so that the image cannot be shared
This bug similar with bug 1033964.
Reporter | ||
Updated•10 years ago
|
blocking-b2g: --- → 2.0?
Ni? Francisco and michalbe for help ~
Flags: needinfo?(mbudzynski)
Flags: needinfo?(francisco)
Assignee | ||
Comment 2•10 years ago
|
||
Thanks for letting us now, I'm on it now.
Flags: needinfo?(mbudzynski)
Flags: needinfo?(francisco)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → mbudzynski
Assignee | ||
Comment 3•10 years ago
|
||
I prepared a gaia part patch that should fix this bug (and sending contacts opened from the dialer as well). `Should` because when trying to share recently received contact via NFC we get a Gecko error:
'[JavaScript Error: "can't access dead object" {file: "jar:file:///system/b2g/omni.ja!/components/nsNfc.js" line: 273}]' when calling method: [nsINfcPeerCallback::peerNotification]" {file: "jar:file:///system/b2g/omni.ja!/components/NfcContentHelper.js" line: 429}]
Can someone responsible for the Gecko side of NFC take a look on this? Garner maybe?
Attachment #8451032 -
Flags: review?(francisco)
Flags: needinfo?(dgarnerlee)
Comment 4•10 years ago
|
||
(In reply to ashiue from comment #0)
> Gaia 6b6b7d7fe829ebea85b01aa4ed44cf5ada366bbe
> Gecko https://hg.mozilla.org/releases/mozilla-aurora/rev/9457a67841b3
> BuildID 20140703160208
> Version 32.0a2
>
> STR:
> 1. Phone A receive contact via NFC (Phone A stay at the contact info page)
> 2. Tap 2 phones together
>
> Expected result:
> Shrinking UI show correctly and the contact can be shared
>
> Actual result:
> No shrinking UI so that the image cannot be shared
>
> This bug similar with bug 1033964.
Can you confirm this is a recent regression ? i am sure we must have tested this case in the first pass of NFC?
Flags: needinfo?(ashiue)
Reporter | ||
Comment 5•10 years ago
|
||
(In reply to bhavana bajaj [:bajaj] [NOT reading Bugmail, needInfo please] from comment #4)
> (In reply to ashiue from comment #0)
> > Gaia 6b6b7d7fe829ebea85b01aa4ed44cf5ada366bbe
> > Gecko https://hg.mozilla.org/releases/mozilla-aurora/rev/9457a67841b3
> > BuildID 20140703160208
> > Version 32.0a2
> >
> > STR:
> > 1. Phone A receive contact via NFC (Phone A stay at the contact info page)
> > 2. Tap 2 phones together
> >
> > Expected result:
> > Shrinking UI show correctly and the contact can be shared
> >
> > Actual result:
> > No shrinking UI so that the image cannot be shared
> >
> > This bug similar with bug 1033964.
>
> Can you confirm this is a recent regression ? i am sure we must have tested
> this case in the first pass of NFC?
I am sorry to say that these test scenarios(this and bug 1033964) do not include in initial NFC test cases. I already added them in moztrap at the assumption that they are really bugs.
Flags: needinfo?(ashiue)
Comment 6•10 years ago
|
||
Looking into it. Definately something slightly off happening. There's a techlost is being fired, but only partially removed from the callback list, and an unregister missed content side (the webapp). Patching that, the app simply doesn't have the ability to P2P share anymore until restarted (i.e. there's no callback found for CheckP2PRegistration, so the ability to share won't be offered.). P2P still works for the Browser.
I'll look at the contact app side too...
Flags: needinfo?(dgarnerlee)
Comment 7•10 years ago
|
||
More of a WIP fix, for the gecko side. This ensures the callback is called, and the underlying internal NFC.ONPEERFOUND and NFC.ONPEERLOST events trigger the corresponding content callbacks. A bare debug message only callback set for onpeerfound works fine without first exiting the comms app.
However, it seems to have some trouble if the lazy loaded function is called gecko side (stale content side mozNfcPeer or related object(s)?).
Flags: needinfo?(mbudzynski)
Updated•10 years ago
|
Target Milestone: --- → 2.0 S6 (18july)
Updated•10 years ago
|
Whiteboard: [p=1]
Updated•10 years ago
|
Blocks: b2g-NFC-2.0
Comment 8•10 years ago
|
||
Hi Garner, I wonder if you could file another bug for your patch. Thanks.
Flags: needinfo?(dgarnerlee)
Assignee | ||
Comment 9•10 years ago
|
||
Garner, I'm not a Gecko dev so I don't think I'm a right person to provide feedback for this patch.
Flags: needinfo?(mbudzynski)
Comment 10•10 years ago
|
||
Bug Created for gecko side cleanup: Bug 1037051.
Updated•10 years ago
|
Attachment #8452060 -
Attachment is obsolete: true
Flags: needinfo?(dgarnerlee)
Comment 11•10 years ago
|
||
This bug is for contacts app. We are going to tracking gecko part of NFC by using bug 1037051.
Component: NFC → Gaia::Contacts
(In reply to Michał Budzyński (:michalbe) from comment #3)
> Created attachment 8451032 [details] [review]
> Gaia part patch
>
> I prepared a gaia part patch that should fix this bug (and sending contacts
> opened from the dialer as well). `Should` because when trying to share
> recently received contact via NFC we get a Gecko error:
> '[JavaScript Error: "can't access dead object" {file:
> "jar:file:///system/b2g/omni.ja!/components/nsNfc.js" line: 273}]' when
> calling method: [nsINfcPeerCallback::peerNotification]" {file:
> "jar:file:///system/b2g/omni.ja!/components/NfcContentHelper.js" line: 429}]
>
> Can someone responsible for the Gecko side of NFC take a look on this?
> Garner maybe?
Hi Michalbe
Can you post your working Gaia branch here and give us steps to reproduce this problem ?
Does this happen all the time?
Thanks
Flags: needinfo?(mbudzynski)
Comment 13•10 years ago
|
||
(In reply to Yoshi Huang[:allstars.chh] from comment #12)
> (In reply to Michał Budzyński (:michalbe) from comment #3)
> > Created attachment 8451032 [details] [review]
> > Gaia part patch
> >
> > I prepared a gaia part patch that should fix this bug (and sending contacts
> > opened from the dialer as well). `Should` because when trying to share
> > recently received contact via NFC we get a Gecko error:
> > '[JavaScript Error: "can't access dead object" {file:
> > "jar:file:///system/b2g/omni.ja!/components/nsNfc.js" line: 273}]' when
> > calling method: [nsINfcPeerCallback::peerNotification]" {file:
> > "jar:file:///system/b2g/omni.ja!/components/NfcContentHelper.js" line: 429}]
> >
> > Can someone responsible for the Gecko side of NFC take a look on this?
> > Garner maybe?
>
> Hi Michalbe
>
> Can you post your working Gaia branch here and give us steps to reproduce
> this problem ?
> Does this happen all the time?
>
> Thanks
Hi Yoshi!
Michal is on holidays, but he added the patch for gaia to enable this. Also the STR are on the first comment here:
1. Phone A receive contact via NFC (Phone A stay at the contact info page)
2. Tap 2 phones together
Thanks!
(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #13)
Sorry I didn't notice he already uploaded his patch.
Flags: needinfo?(mbudzynski)
Comment 15•10 years ago
|
||
I've used the patch uploaded by Michał and added additional logging in Gecko. This is the logcat output from the following scenario:
1. Share contact from other device to Flame. (This is successful)
2. Not closing the current screen try to share received contact to different device. (This fails, shrinking UI is not visible)
3. Press the home button. Open contacts app, open previously received contact and try to share it with other device. (shrinking UI is visible, this is successful)
In the log we can see that patch uploaded by Michał works, callback registration after point 1. reaches Nfc.js in Gecko (line 8595):
> I/Gecko ( 282): -*- Nfc: Received 'NFC:RegisterPeerTarget' message from content process
While doing point 2. NfcManager discovers the other device and tries to get the manifest url of the current app from Shrinking UI. Log print outs starting from line 8817 show that the manifest url which NfcManager gets from Shrinking UI is null (it shouldn't be even passed to Gecko, but still is):
>I/GeckoDump( 282): [DEBUG] SYSTEM NFC: Technology Discovered: {"type":"techDiscovered","techList":["P2P"],"records":null,"sessionToken":"{9d3ed0ee-971f-44eb-be59-4fc751741909}"}
>I/GeckoDump( 282): [DEBUG] SYSTEM NFC: KM checking for manifest
>I/Gecko ( 282): -*- Nfc DOM: KM this is the manifest URL: null
As a result the Shrinking UI animation does not start (line 8841):
> I/GeckoDump( 282): [DEBUG] SYSTEM NFC: Error checking P2P Registration: false
This is because after the successful share in point 1. the contact details screen is an Activity not the Contact app itself, so Shrinking UI does not have the manifest set up. Alison has pointed out in comment 1 that this issue is similar to bug 1033964 and this is actually the same problem which was identified in bug 1033964 comment 7.
Still I could not replicate the problem which Michał had in comment 3. But I think this is a different issue not related with the problem originally raised by Alison.
Comment 16•10 years ago
|
||
Wesley, according to comment 15 and your comment here: bug 1033964 comment 17, I think this bug should not block 2.0+. What do you think?
Flags: needinfo?(whuang)
Comment 17•10 years ago
|
||
I do agree with Krzysztof, it a rarely scenario that can be fix by just getting out from the activity, in a flow that actually doesn't make much sense.
I wouldn't block the release here, and try to properly fix it in 2.1
Cheers
F.
Comment 18•10 years ago
|
||
I'm trying to repo the bug again with just this patch applied. What I see is:
1) Share from other device to Flame device. (Flame receives contact).
2) Remove, and re-establish P2P link. (Flame still on newly received contact screen).
3) From Flame, share to other device.
What happens:
On the Share/Shrink UI, it dismisses the originally shared contact, back to the contact screen, new contact is not shared.
What probably should happen:
The received contact screen should dismiss, and share the current contact.
Comment 19•10 years ago
|
||
(In reply to Garner Lee from comment #18)
> I'm trying to repo the bug again with just this patch applied. What I see is:
>
> 1) Share from other device to Flame device. (Flame receives contact).
> 2) Remove, and re-establish P2P link. (Flame still on newly received contact
> screen).
> 3) From Flame, share to other device.
>
> What happens:
> On the Share/Shrink UI, it dismisses the originally shared contact, back to
> the contact screen, new contact is not shared.
>
> What probably should happen:
> The received contact screen should dismiss, and share the current contact.
^^ The received contact screen should NOT dismiss, and share the current contact.
Comment 20•10 years ago
|
||
This is similar to bug 1033964, using Activity instead of the real app.
Per comment in https://bugzilla.mozilla.org/show_bug.cgi?id=1033964#c17 , let's nominate it as 2.1? to triage later.
blocking-b2g: 2.0+ → 2.1?
Flags: needinfo?(whuang)
Reporter | ||
Updated•10 years ago
|
QA Whiteboard: [COM=NFC]
Comment 22•10 years ago
|
||
Wait, this partial fix (to re-apply the callback to be able to share again without exiting) isn't in 2.0?
Updated•10 years ago
|
QA Whiteboard: [COM=NFC] → [COM=NFC], [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Whiteboard: [p=1] → [p=1], [2.0-flame-test-run-3]
Updated•10 years ago
|
status-b2g-v2.0:
--- → affected
Updated•10 years ago
|
QA Whiteboard: [COM=NFC], [QAnalyst-Triage?] → [COM=NFC], [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Updated•10 years ago
|
Target Milestone: 2.0 S6 (18july) → 2.1 S1 (1aug)
Comment 23•10 years ago
|
||
I recall that UX made decision that it should launch contact app instead of inline activity.
ni? Juwei to reconfirm this.
Flags: needinfo?(jhuang)
Updated•10 years ago
|
QA Whiteboard: [COM=NFC], [QAnalyst-Triage+] → [COM=NFC], [QAnalyst-Triage+][lead-review+]
Comment 24•10 years ago
|
||
As an agreement with Wesley & Greg in comment 9 on bug 1033964, we are now looking for the possibility of solution 1: launch contact app directly from notification.
Flags: needinfo?(jhuang)
Comment 25•10 years ago
|
||
Comment on attachment 8451032 [details] [review]
Gaia part patch
Great job!
r+, relaunched the ci work in gaia-try, please land once gree.
Attachment #8451032 -
Flags: review?(francisco) → review+
Updated•10 years ago
|
Target Milestone: 2.1 S1 (1aug) → 2.1 S2 (15aug)
Updated•10 years ago
|
Blocks: b2g-NFC-2.1
Comment 26•10 years ago
|
||
Not a release blocker but it's a most-wanted from UX for 2.1.
blocking-b2g: 2.1? → backlog
Whiteboard: [p=1], [2.0-flame-test-run-3] → [p=1], [2.0-flame-test-run-3] [priority]
Comment 27•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Reporter | ||
Updated•10 years ago
|
QA Whiteboard: [COM=NFC], [QAnalyst-Triage+][lead-review+] → [QAnalyst-Triage+][lead-review+]
Updated•10 years ago
|
blocking-b2g: backlog → ---
tracking-b2g:
--- → backlog
You need to log in
before you can comment on or make changes to this bug.
Description
•