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

NEW
Unassigned

Status

Firefox OS
Gaia::Contacts
P4
normal
4 years ago
3 years ago

People

(Reporter: Anshul, Unassigned)

Tracking

unspecified
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(tracking-b2g:backlog)

Details

(Reporter)

Description

4 years ago
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.

Updated

4 years ago
Assignee: nobody → jmcf

Comment 1

4 years ago
Anshul,

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

thanks!
Flags: needinfo?(anshulj)

Comment 2

4 years ago
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.
Flags: needinfo?(anshulj)

Comment 3

4 years ago
(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.
Component: Gaia::Contacts → RIL
Summary: Canceling import contacts doesn't send cancel request to the framework → Canceling import contacts doesn't send cancel request to the gecko
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

Updated

4 years ago
Assignee: jmcf → nobody

Updated

4 years ago
Depends on: 935398

Comment 5

4 years ago
(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
(Reporter)

Comment 6

4 years ago
(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?

Updated

3 years ago
Blocks: 1025317
No longer blocks: 1011657
Flags: needinfo?(kchang)

Comment 7

3 years ago
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.
Flags: needinfo?(kchang) → needinfo?(skamat)

Comment 8

3 years ago
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.
Flags: needinfo?(skamat)

Comment 9

3 years ago
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?
Flags: needinfo?(htsai)
(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.
Flags: needinfo?(htsai)
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?
Flags: needinfo?(skamat)

Comment 12

3 years ago
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.
Flags: needinfo?(htsai)
Flags: needinfo?(anshulj)
(Reporter)

Updated

3 years ago
No longer blocks: 1025317
Flags: needinfo?(anshulj)
(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.
Flags: needinfo?(htsai)

Updated

3 years ago
Blocks: 1063044

Comment 14

3 years ago
Given risks and timing, OK for moving out of 2.1 for now and making it 2.2nom
feature-b2g: --- → 2.2?
Flags: needinfo?(skamat)

Comment 15

3 years ago
[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.
blocking-b2g: --- → 2.0?
Flags: needinfo?(whuang)
Flags: needinfo?(vchen)
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.
blocking-b2g: 2.0? → backlog
Flags: needinfo?(whuang)
Bug 935398 is for the gecko work, and this is for gaia using the new API from 935398, changing component accordingly.
Component: RIL → Gaia::Contacts
clear my ni since we already move this one forward
Flags: needinfo?(vchen)
No longer blocks: 1063044
[Tracking Requested - why for this release]:
not in 2.2 must-have scope.
feature-b2g: 2.2? → ---
tracking-b2g: --- → ?
(Assignee)

Updated

3 years ago
blocking-b2g: backlog → ---
Priority: -- → P4
You need to log in before you can comment on or make changes to this bug.