1021962 - Canceling import contacts doesn't send cancel request to the gecko

Status: RESOLVED WONTFIX

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

Description

[Anshul]

STR
1. Go to the Contacts app->Settings->Import Contacts->SIM
2. When the import is still happening, click on the cancel button


Observed: Import UI disappears as expected but the framework doesn't know that user canceled the operation and so could continue to read the contacts from ICC card. This allows user to attempt an import again while the previous is going on causing all sorts of issues.

Expected: Gaia should send a cancel request to framework.

Comment 1

[Jose Manuel Cantera]

Anshul,

If you tell me what API call should I do I can fix this issue 

thanks!

Comment 2

[Nivi]

Hi Jose,

There is no existing API. We are asking for a support for a message something just like "RIL:ReadIccContacts" - "RIL:CancelIccContactsOp" (for generic export/import) or "RIL:CancelRead/UpdateIccContacts" (for import/export). When we get this new message on clicking Cancel from the UI, we will handle that correctly and actually cancel the import which is the correct behavior.

Please note that this is merely a suggestion (one way of solving this). I am not very acquainted with the gaia/gecko code but we need to somehow get notified when Cancel is clicked on the UI during import/export contacts.

Please let me know if you have any more questions.

Thanks,
Nivi.

Comment 3

[Jose Manuel Cantera]

(In reply to Nivi from comment #2)
> Hi Jose,
> 
> There is no existing API. We are asking for a support for a message
> something just like "RIL:ReadIccContacts" - "RIL:CancelIccContactsOp" (for
> generic export/import) or "RIL:CancelRead/UpdateIccContacts" (for
> import/export). When we get this new message on clicking Cancel from the UI,
> we will handle that correctly and actually cancel the import which is the
> correct behavior.
> 

Yes, but you would need to expose to the iccManager API, the cancel request, something like iccManager.cancelRead() 

> Please note that this is merely a suggestion (one way of solving this). I am
> not very acquainted with the gaia/gecko code but we need to somehow get
> notified when Cancel is clicked on the UI during import/export contacts.

Yes, through that API call I have suggested. Adding Yoshi to the bug as he could make suggestions as well 

> 
> Please let me know if you have any more questions.
> 
> Thanks,
> Nivi.

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Bug 935398 is to replace DOMRequest with DOMCursor to readContacts.

However for updateContact might have some concern on it, while the RIL stack is updating the phonebook fields and then user clicks 'Cancel', that seems to leave the content to a unconsistent state, i.e. name:Yoshi -> name:Yo

Comment 5

[Jose Manuel Cantera]

(In reply to Yoshi Huang[:allstars.chh] from comment #4)
> Bug 935398 is to replace DOMRequest with DOMCursor to readContacts.

So when that bug lands and Gaia is properly updated, the issue described by this bug will be fixed, right?

> 
> However for updateContact might have some concern on it, while the RIL stack
> is updating the phonebook fields and then user clicks 'Cancel', that seems
> to leave the content to a unconsistent state, i.e. name:Yoshi -> name:Yo

I think Gaia, should cancel the process safely ensuring that all pending requests have been resolved

Comment 6

[Anshul]

(In reply to Yoshi Huang[:allstars.chh] from comment #4)
> Bug 935398 is to replace DOMRequest with DOMCursor to readContacts.
> 
> However for updateContact might have some concern on it, while the RIL stack
> is updating the phonebook fields and then user clicks 'Cancel', that seems
> to leave the content to a unconsistent state, i.e. name:Yoshi -> name:Yo
Yoshi, whats the ETA for bug 935398?

Comment 7

[Ken Chang[:kenkai]]

We just lock down the scope of 2.1. However, this bug is not in the scope of 2.1. ni?Sandip to know if it really need to be in the scope of 2.2.

Comment 8

[Sandip Kamat]

Hi Ken, Checked with Anshul and tried on Flame. Looks like bad impact if the use cancels contact import and retries (which is a common use case). Let's discuss offline how we can get this in.

Comment 9

[Sandip Kamat]

After talking with Ken further and reviewing the scope of changes, it looks beyond 2.1 timeframes. Hsinyi - lets discuss this for 2.2 scoping?

Comment 10

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Sandip Kamat from comment #9)
> After talking with Ken further and reviewing the scope of changes, it looks
> beyond 2.1 timeframes. Hsinyi - lets discuss this for 2.2 scoping?

For cancelling "import" contacts, changing API "DOMRequest mozIcc.readContacts" into "DOMCursor mozIcc.readContacts" (bug 935398) seems the right direction. And work effort is huge [p=13+].

Overall we need:
1) add a new internal API, nsIIccReadContactsCursorCallback.idl and its implementation, new files: nsIIccReadContactsCursorCallback.idl, IccReadContactsCursorCallback.h & *.cpp
2) change DOM implementation to adopt cursor concept, touched files: MozIcc.webidl, Icc.h, Icc.cpp
3) change ril implementation to adopt cursor concept, touched files: nsIIccProvider.idl, RILContentHelper.js, RadioInterfaceLayer.js, ril_worker.js
4) Gaia work: use the new API

Just for your information, we took 2 more months on moving DOMRequest into DOMCursor for SMS API.

Comment 11

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Hi Sandip,
If this was confirmed to move to 2.2, can we flag the feature-b2g/blocking-b2g, and remove it from CAF metabug?

Comment 12

[Ken Chang[:kenkai]]

Anshul, it is hard to have this in 2.1. I wonder if you could remove it form 2.1 CAF meta bug.
Hsinyi, let's find someone working on this from now.

Comment 13

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Ken Chang[:ken] from comment #12)
> Anshul, it is hard to have this in 2.1. I wonder if you could remove it form
> 2.1 CAF meta bug.
> Hsinyi, let's find someone working on this from now.

Ken, okay, let me talk with the team.

Comment 14

[Sandip Kamat]

Given risks and timing, OK for moving out of 2.1 for now and making it 2.2nom

Comment 15

[Jack Liu]

[Blocking Requested - why for this release]: This bug reported on 1.3 but since 1.3 has already closed, so we need to fix it in 2.0 maintenance release.

Comment 16

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

This was discussed with product PM to put into 2.2 planning as a feature.
In reality it's risky to implement for 2.1/2.0 now so I'm removing the blocking flag.

Comment 17

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Bug 935398 is for the gecko work, and this is for gaia using the new API from 935398, changing component accordingly.

Comment 18

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

clear my ni since we already move this one forward

Comment 19

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

[Tracking Requested - why for this release]:
not in 2.2 must-have scope.

Comment 20

[BMO Automation]

Firefox OS is not being worked on