1000014 - B2G RIL: Dispatch 'cfstatechange' only when the setCallForwarding request is success

Status: RESOLVED DUPLICATE of bug 843452

(Firefox OS Graveyard :: RIL, defect)

Description

[Edgar Chen [:edgar] (OOO 08/03 ~ 08/07)]

Currently, if someone changes the callforwarding configuration by calling setCallForwarding() or sending MMI, we will dispatch a 'cfstatechange' event no matter the request is success or not. And gaia needs to check |success| [1] attribute to filter out the error one.

This behavior looks weird to me, I am thinking about dispatching 'cfstatechange' event *only* when the setCallForwarding request is success. In this way, gaia don't need to worry about the error case and we also can get rid of the |success| attribute [1].

[1] http://dxr.mozilla.org/mozilla-central/source/dom/webidl/CFStateChangeEvent.webidl#11-14
[2] https://github.com/mozilla-b2g/gaia/blob/fcb82039b884053a2a246290b718e48f45085640/apps/system/js/call_forwarding.js#L133-L138

Comment 1

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

(In reply to Edgar Chen [:edgar][:echen] from comment #0)
> Currently, if someone changes the callforwarding configuration by calling
> setCallForwarding() or sending MMI, we will dispatch a 'cfstatechange' event
> no matter the request is success or not. And gaia needs to check |success|
> [1] attribute to filter out the error one.
> 
> This behavior looks weird to me, I am thinking about dispatching
> 'cfstatechange' event *only* when the setCallForwarding request is success.
> In this way, gaia don't need to worry about the error case and we also can
> get rid of the |success| attribute [1].
> 

Sounds good to me as well, if gaia is also fine with the change :)

> [1]
> http://dxr.mozilla.org/mozilla-central/source/dom/webidl/CFStateChangeEvent.
> webidl#11-14
> [2]
> https://github.com/mozilla-b2g/gaia/blob/
> fcb82039b884053a2a246290b718e48f45085640/apps/system/js/call_forwarding.
> js#L133-L138

Comment 2

[Edgar Chen [:edgar] (OOO 08/03 ~ 08/07)]

(In reply to Edgar Chen [:edgar][:echen] from comment #0)
> Currently, if someone changes the callforwarding configuration by calling
> setCallForwarding() or sending MMI, we will dispatch a 'cfstatechange' event
> no matter the request is success or not. And gaia needs to check |success|
> [1] attribute to filter out the error one.
> 
> This behavior looks weird to me, I am thinking about dispatching
> 'cfstatechange' event *only* when the setCallForwarding request is success.
> In this way, gaia don't need to worry about the error case and we also can
> get rid of the |success| attribute [1].
> 
> [1]
> http://dxr.mozilla.org/mozilla-central/source/dom/webidl/CFStateChangeEvent.
> webidl#11-14
> [2]
> https://github.com/mozilla-b2g/gaia/blob/
> fcb82039b884053a2a246290b718e48f45085640/apps/system/js/call_forwarding.
> js#L133-L138

Hi jaoo,

May I have your opinion about this proposal from Gaia point of view?

Thank you.

Comment 3

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Edgar Chen [:edgar][:echen] from comment #2)
> May I have your opinion about this proposal from Gaia point of view?

Sure, a first glance It seems the change is safe and won't bring any trouble. Please go ahead. Once you have the gecko patch I could provide a gaia patch to test the whole change and make sure everything works fine.

Comment 4

[Edgar Chen [:edgar] (OOO 08/03 ~ 08/07)]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #3)
> (In reply to Edgar Chen [:edgar][:echen] from comment #2)
> > May I have your opinion about this proposal from Gaia point of view?
> 
> Sure, a first glance It seems the change is safe and won't bring any
> trouble. Please go ahead. Once you have the gecko patch I could provide a
> gaia patch to test the whole change and make sure everything works fine.

Thanks for your reply.

Because the proposal touches both Gecko and Gaia side, so we could have 3 steps to achieve this without breaking any thing.
1). Gecko dispatch the event only when request is success.
    (Still keeps the |success| attribute and the value is always true)
2). Gaia don't need to check |success| because gecko always dispatch event with |success| is true. 
3). Remove |success| attribute.

This bug will focus on 1).

Thank you.

Comment 5

[Edgar Chen [:edgar] (OOO 08/03 ~ 08/07)]

Before doing this changes, I would like to have test case for callforwarding first. :)

Comment 6

[Edgar Chen [:edgar] (OOO 08/03 ~ 08/07)]

Actually this was fixed in bug 843452 already [1][2].

[1] dxr.mozilla.org/mozilla-central/source/dom/mobileconnection/gonk/MobileConnectionService.js#722-735

Comment 7

[Edgar Chen [:edgar] (OOO 08/03 ~ 08/07)]

(In reply to Edgar Chen [:edgar][:echen] from comment #6)
> Actually this was fixed in bug 843452 already [1][2].
> 
> [1]
> dxr.mozilla.org/mozilla-central/source/dom/mobileconnection/gonk/
> MobileConnectionService.js#722-735

[2] http://dxr.mozilla.org/mozilla-central/source/dom/mobileconnection/gonk/MobileConnectionService.js#523-528

> 
> *** This bug has been marked as a duplicate of bug 843452 ***