[Contacts]Exporting contacts through Bluetooth results in a Failed message

VERIFIED FIXED in 2.2 S7 (6mar)

Status

VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: JMercado, Assigned: hola)

Tracking

({regression})

unspecified
2.2 S7 (6mar)
ARM
Gonk (Firefox OS)
regression
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.1 unaffected, b2g-v2.2 verified, b2g-master verified)

Details

(Whiteboard: [3.0-Daily-Testing][v2.2-nexus-5-l], URL)

Attachments

(3 attachments)

Created attachment 8566285 [details]
Failed export Logcat

Description:
When exporting a contact through Bluetooth a message saying that the file transfer failed will pop up.  The message states to check that Bluetooth settings are correct, but occurs regardless of Bluetooth setup.

Prerequisites:
1) Have at least one contact on the phone.

Repro Steps:
1) Update a Flame to 20150218010226
2) Open the contacts app.
3) Go to export under the contact settings and select Bluetooth.
4) Select at least one contact and press Export.


Actual:
An error message pops up saying that the transfer Failed.


Expected:
The user is brought to a Bluetooth setup page or a paired device page.

Environmental Variables:
Device: Flame 3.0 KK (Full Flash) (319MB)
Build ID: 20150218010226
Gaia: 82f286f10a41aab84a0796c89fbefe67b179994b
Gecko: 9696d1c4b3ba
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Repro frequency: 10/10
See attached: Video, Logcat
This issue DOES occur on Flame 2.2.

Results: The user is shown an error message when exporting.

Environmental Variables:
Device: Flame 2.2
BuildID: 20150218002515
Gaia: da509caa7395d3d090ce973e8de082b4680a590d
Gecko: 96da179a7d3a
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0



This issue does NOT occur on Flame 2.1.

Results: The user is taken to Bluetooth setup or a paired device page when exporting.

Environmental Variables:
Device: Flame 2.1 KK (Full Flash) (319MB)
Build ID: 20150217001459
Gaia: e8eba437af02820f74d122aec83b6001df6f89e3
Gecko: 9d04f9149ca4
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 34.0 (2.1)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Let's find the regression window here.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Keywords: regression, regressionwindow-wanted
Whiteboard: [3.0-Daily-Testing]
QA Contact: jmercado
The changes for bug 1130654 seems to have caused this issue.

B2g-inbound Regression Window

Last Working 
Environmental Variables:
Device: Flame 3.0
BuildID: 20150213004957
Gaia: 71defd569502274e877b56094e3a44d776033120
Gecko: 5f83e77bfb40
Version: 38.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

First Broken 
Environmental Variables:
Device: Flame 3.0
BuildID: 20150213010359
Gaia: dbbb9b0be9c30b96146cd1259ed6d6f8c8eaadf3
Gecko: 0243d54ec101
Version: 38.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Last Working gaia / First Broken gecko - Issue does NOT occur
Gaia: 71defd569502274e877b56094e3a44d776033120
Gecko: 0243d54ec101

First Broken gaia / Last Working gecko - Issue DOES occur
Gaia: dbbb9b0be9c30b96146cd1259ed6d6f8c8eaadf3
Gecko: 5f83e77bfb40

Gaia Pushlog: https://github.com/mozilla-b2g/gaia/compare/71defd569502274e877b56094e3a44d776033120...dbbb9b0be9c30b96146cd1259ed6d6f8c8eaadf3
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Keywords: regressionwindow-wanted
[Blocking Requested - why for this release]:

This is a regression and obvious broken functionality so nominating 2.2?

Jose, can you take a look at this please? Looks like the landing for bug 1130654
blocking-b2g: --- → 2.2?
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker) → needinfo?(jmcanterafonseca)
blocking-b2g: 2.2? → 2.2+
Blocks: 1130654
(Assignee)

Updated

4 years ago
Assignee: nobody → hola
Created attachment 8568011 [details] [review]
[gaia] ADLR-es:bluetooth > mozilla-b2g:master
(Assignee)

Comment 6

4 years ago
Comment on attachment 8568011 [details] [review]
[gaia] ADLR-es:bluetooth > mozilla-b2g:master

A new kind of activity was registered on the manifest but the logic of Bluetooth app knew nothing about it. Fixed to be treated as a share activity, like it was before.
Attachment #8568011 - Flags: review?(jmcf)

Updated

4 years ago
Flags: needinfo?(jmcanterafonseca)
Comment on attachment 8568011 [details] [review]
[gaia] ADLR-es:bluetooth > mozilla-b2g:master

Hi Adrian,

Please find a Bluetooth peer to perform this review

thanks
Attachment #8568011 - Flags: review?(jmcf)
Hi Adrian, could you provide an estimation for this?
(Assignee)

Comment 9

4 years ago
Comment on attachment 8568011 [details] [review]
[gaia] ADLR-es:bluetooth > mozilla-b2g:master

Asking review to a Devices (bluetooth) peer.
Attachment #8568011 - Flags: review?(jaliu)
(Assignee)

Comment 10

4 years ago
Hi Francisco, it's already fixed. We are waiting for a review from a Bluetooth peer as Jose Manuel requested.
Comment on attachment 8568011 [details] [review]
[gaia] ADLR-es:bluetooth > mozilla-b2g:master

Hi Ian,
Would you mind to review this Gaia patch ?
Attachment #8568011 - Flags: review?(jaliu) → review?(iliu)
Sure. When is the new activity name 'export' created? Why I don't join the discussion before.. 

Per bug 1130654, looks like the bug is fixed via creating new activity name 'export'. The activity only services for specific user story of Contact app. Is there no way to re-use 'share' activity via filter in Email/Message?

IMO, the naming 'export' is a little bit same with 'share'. I cannot distinguish from them. 

"export": {
  "filters": {
    "type": "text/vcard",
    "blobs": { "required":true }
  },
  "disposition": "inline",
  "returnValue": true,
  "href": "/transfer.html"
}

From the definition of manifest, it means Bluetooth app only services 'export' request for 'type' = 'text/vcard'. It won't have any regression, I think. But could we have a more clear name to figure out that is needed service from Bluetooth(The user story is very clear to use Bluetooth for share/export something). In this point, I would like to ask Fabrice and David for some suggestion before I review the patch. Especially, this is block+ bug. Thanks.
Flags: needinfo?(hola)
Flags: needinfo?(francisco)
Flags: needinfo?(fabrice)
Flags: needinfo?(dflanagan)
(Assignee)

Comment 14

4 years ago
I think Jose Manuel, who made the patch for bug 1130654, will be more helpful speaking about why the patch is that way ;)
Flags: needinfo?(hola) → needinfo?(jmcf)
I agree with Ian that using a single 'share' activity and filtering with the |"type": "text/vcard"| should be enough there.
Flags: needinfo?(fabrice)
Thanks for asking, Ian.

If I understand correctly, Fabrice is saying that we should revert bug 1130654 so that we only have the share activity. He may be missing, however, that the contacts app has a regular "share" function for single contacts, and that we do want (I believe) the email and messages app to be able to receive text/vcard files that are shared in that way.

So if we need to distinguish sharing from exporting, one obvious way to do it is to define a new activity, as bug 1130654 did (incompletely, causing this bug). Defining new activities is not something we should do lightly, though, so I understand the concern here.

But if we are going to try to use the share activity for both sharing and exporting an we want the list of handlers to be different in the two cases, then we need to add more information to the activity request, and we need to modify email and messages to filter based on that information so that they do not appear in the list. The contacts app would have to add "bluetooth_only: true" to the request, and then email and contacts (and all future apps) would have to include filters to ignore activity requests with that property set.  (I always forget how activity filters work, so I may have some details on that wrong.)  That seems clearly worse than defining a new activity name.

On the other hand, there is a fundamental UX issue here. (Which doesn't surprise me because in my experience our UX team and our engineers have fundamentally different ideas about how activities work or how they should work. Basically UX designers just never get it.) If the button in the app says "Export via Bluetooth" and the implementation of that button involves an activity request, it will never work.  Any app that registers an activity handler for name "export" and type "text/vcard" will appear on that list.  That's just the way activities work.

Here are my recommendations. I prefer the first but suspect that the second is the easier one to get done for 2.2:

1) Revert bug 1130654 and change the export UX, so that when the user exports contacts they see the choices "SIM, Memory card, and Share" instead of "SIM, Memory card, and Bluetooth".  By just changing the text, the problem goes away. Or maybe these buttons could be changed to have two lines.  For the SIM button, the second line would display the phone number in small font.  For the Share button, the second line would say something like "Bluetooth, Messages, Email, etc." Setting needinfo for Carrie for her opinion on this suggestion.

2) Proceed with a new activity as bug 1130654 and the patch attached here do, but instead of using the generic "export", rename it to indicate that it is an engineering hack and not a general purpose thing. I'd call it "share-via-bluetooth-only" or something like that.
Flags: needinfo?(dflanagan) → needinfo?(cawang)
David, provided the requested info. clearing my ni
Flags: needinfo?(jmcf)
Thanks David,

you provided a great feedback on what's happening right now.

In the current states for 2.2, I don't think we are in a good moment to try to go for solution 1. It will require major work and risk of regressions is  high.

As well, I understand that creating a new activity verb is something that shouldn't be necessary, so my suggestion is to take solution 2, for 2.2 and have a better and, if possible, unified way of doing this for v3.
Flags: needinfo?(francisco)
(Assignee)

Comment 19

4 years ago
The attached PR has been updated according to David's solution 2, following comment 18.

Thanks, everyone!
We didn't plan to have mms/email sharing from here, so it's not a regression. In addition, I think it works fine with solution 2 under the strict timeline for 2.2, but it's even better to have more sharing options here in the future. So I'd say let's adopt #2 for now and I'll refine it in v3. Thanks!
Flags: needinfo?(cawang)
Thanks for everyone's input here. Especially David's guidance, it's more clear how we can use the activity name/filter. Per above conclusion, let's adopt solution 2 to create new activity 'share-via-bluetooth-only' service.
Comment on attachment 8568011 [details] [review]
[gaia] ADLR-es:bluetooth > mozilla-b2g:master

Adrián, I r+ the patch with completed commit message. Please revise it before merge. Thanks.

'in ...' -> 'What you have done in this patch, r=reviewer'
Attachment #8568011 - Flags: review?(iliu) → review+
(Assignee)

Comment 23

4 years ago
Commit message amended. Marking as checkin-needed.
Keywords: checkin-needed
Keywords: checkin-needed
Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.

Updated

4 years ago
Component: Gaia::Contacts → Gaia::Bluetooth File Transfer
Keywords: checkin-needed
Keywords: checkin-needed
Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
Keywords: checkin-needed
Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.
(Assignee)

Updated

4 years ago
Component: Gaia::Bluetooth File Transfer → Gaia::Settings
Keywords: checkin-needed
Keywords: checkin-needed
Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.
landed in master:

https://github.com/mozilla-b2g/gaia/commit/0e5c608dc8dd1f8f9230825750e92c728ba28a98
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Comment 29

4 years ago
Comment on attachment 8568011 [details] [review]
[gaia] ADLR-es:bluetooth > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 1130654
[User impact] if declined: User will be unable to share contacts using bluetooth from Contacts app.
[Testing completed]: Manual testing, sharing by bluetooth works again as expected.
[Risk to taking this patch] (and alternatives if risky): Low risk, few changes.
[String changes made]: None.
Attachment #8568011 - Flags: approval-gaia-v2.2?
status-b2g-master: affected → fixed
Target Milestone: --- → 2.2 S7 (6mar)
Duplicate of this bug: 1139322
Attachment #8568011 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Keywords: verifyme
Verified fix for flame on 3.0 devices:
Results: Phone successfully exports contacts via bluetooth to another device/phone.

--------------------------------------------------
Environmental Variables:
Device: Flame 3.0
BuildID: 20150305010212
Gaia: eff3321ab4e65da3f906688ebb55ddf1e93d9452
Gecko: 56492f7244a9
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 39.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0
--------------------------------------------------
Status: RESOLVED → VERIFIED
Per Comment 32,set "b2g-v3.0" as verified.
status-b2g-master: fixed → verified

Comment 34

4 years ago
This issue CAN be repro on Nexus 5_2.2,can NOT be reprp on Nexus 5_3.0.

Build version:

N5_2.2 build:

Build ID               20150305002528
Gaia Revision          89af288bad6751248ff84504fa898206fee127fe
Gaia Date              2015-03-04 18:00:05
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/6d8d294aa8f3
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.0
Firmware(Incremental)  eng.cltbld.20150305.041802
Firmware Date          Thu Mar  5 04:18:17 EST 2015
Bootloader             HHZ12d

N5_3.0 build:

Build ID               20150305160202
Gaia Revision          7a91c16bfa348be8b25e09719178efa051512988
Gaia Date              2015-03-05 19:20:41
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/0189941a3fd5
Gecko Version          39.0a1
Device Name            hammerhead
Firmware(Release)      5.0
Firmware(Incremental)  eng.cltbld.20150305.191827
Firmware Date          Thu Mar  5 19:18:43 EST 2015
Bootloader             HHZ12d
Whiteboard: [3.0-Daily-Testing] → [3.0-Daily-Testing][v2.2-nexus-5-l]
This bug has been successfully verified on latest Flame v2.2.
See attachment: verified_v2.2.mp4
Reproduce rate: 0/5

Flame 2.2 build:
Build ID               20150308002503
Gaia Revision          166491b92278dc9e648f8d49ab02d9ca00d74421
Gaia Date              2015-03-06 18:26:27
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/a48af0b5a6e4
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150308.052515
Firmware Date          Sun Mar  8 05:25:25 EDT 2015
Bootloader             L1TC000118D0
-----------------------------------------------------------------------------------
Leaving "verifyme" for N5.
status-b2g-v2.2: fixed → verified
This bug has been successfully verified on latest Nexus 5_v2.2.It can export contact successfully via bluetooth.
Reproduce rate: 0/5

Nexus 5_v2.2:
Build ID               20150308002503
Gaia Revision          166491b92278dc9e648f8d49ab02d9ca00d74421
Gaia Date              2015-03-06 18:26:27
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/a48af0b5a6e4
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.0
Firmware(Incremental)  eng.cltbld.20150308.052750
Firmware Date          Sun Mar  8 05:28:06 EDT 2015
Bootloader             HHZ12d
Keywords: verifyme
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.