1034421 - [Costcontrol] Costcontrol Widget is not showing proper message under airplane mode scenario without simcard

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Cost Control, defect)

Description

[vsireesha246]

Costcontrol Widget is not showing proper message under airplane mode scenario in V1.4 version.

STR:

1.In V1.4 branch apply the patch https://github.com/mozilla-b2g/gaia/pull/21330
(This patch is related to Bug-960500 for Version 1.4)
2.After make..trun on/off the airplane mode without FTE configuration in costcontrol app.
3.The Costcontrol widget will not shown sometime the proper message or sometimes it will not refresh.(Close and reopen the utility tray the widget message will be refreshed)

The same bug is not present in Master.

So for V1.4 version is there any other dependent patch for this bug?

Platform version:30.0
Build Identifier:20140702181851

Comment 1

[vsireesha246]

Mai as discussed in IRC would you please help on this.

Comment 2

[vsireesha246]

Wayne Would you please help on this.

Comment 3

[Wayne Chang [:wchang]]

:mai would be the best person to follow up here.

I assume Master/2.0 are both working correctly?

Comment 4

[vsireesha246]

Hi Wayne,

In Master/2.0 the behavior of utility tray is different than V1.4.
Once we turn on/off the airplane mode in Master/2.0 the utility tray hides or closes automatically.
Is it correct behavior of utility tray?because this i observed recently.
So once utility tray hides, again opens it, the widget message refreshed.( like we close and open in V1.4)

But as per my opinion in Master/2.0 some cases are failing like
1.Insert no SIM in device
2.Turn ON/OFF the airplane mode
3.Check the widget message..its showing wrong

All these issues i discussed yesterday with Mai in IRC.

Thanks You Wayne..

Comment 5

[vsireesha246]

Attachment: Bug_1034421.patch

Hi Mai,

Would you please review this patch.
I added some other condition on top of you patch(https://github.com/mozilla-b2g/gaia/pull/21330)

I tested in V1.4 branch and all conditions are working fine.

If your ok with this patch i will create PR for it.

Thanks..
Sireesha

Comment 6

[Marina Rodríguez [:mai]]

Comment on attachment 8450791 [details] [diff] [review]
Bug_1034421.patch

Review of attachment 8450791 [details] [diff] [review]:
-----------------------------------------------------------------

Great work, but I think it's not the correct solution.

::: apps/costcontrol/js/widget.js
@@ +467,5 @@
> +              }
> +            );
> +            showSimError('airplane-mode');
> +          }
> +        } else if (state === 'disabled') {

This part will not work fine with DS device. When the airplanemode is disabled you have to reload the icc (the setting of data traffic can be changed when the airplanemode is enabled). 
Common.loadDataSIMIccId(function _onDisabledAirplaneMode() {
  if (initialized) {
    checkCardState(Common.dataSimIccId);
    updateUI();
  } else {
    checkSIMStatus();
  }
},function _errorNoSim() { showSimError('no-sim2');} );


Moreover, I was concern by the possibility of a rece condition between the code execute by the events iccdetectec and statechange (when the state is disabled).


Maybe the solution has to be focus on change the message showed by the widget when the airplanemode is disabled. Something like:
} else if (state === 'disabled') {
  updateSimErrorMessage('no-sim2');
}

Comment 7

[Marina Rodríguez [:mai]]

On master branch it's not posible testing the patch because the utility tray is closed when the airplanemode button is pressed. And I can't see any error on the logcat report.

Comment 8

[vsireesha246]

Attachment: Bug_1034421_Modified.patch

Hi Mai,

I modified the patch for V1.4 as per your first comments.
Like split the function as mentioned in your second comment will not work in all conditions i feel.

So,please check it and let me know your review comments and i don't have DS device.

Thanks..
Sireesha

Comment 9

[Marina Rodríguez [:mai]]

Attachment: Patch for v1.4 to Sireesha

This patch combine a rebase for v1.4 of the patch 960500 + the patch for this bug. It's only for testing purposes

Comment 10

[Marina Rodríguez [:mai]]

Attachment: Patch For Master branch

This patch resolves the following problem on master branch.

Scenario:
Device without SIMCard

STR:

1. Enable airplanemode
2. Disable ariplanemode
3. Slide down the utility tray.

Expected:
Message: No simcard

Current:
Message: Airplanemode enabled.

This occurs because the widget is waiting for the icc detected event to refresh its ui.

Regards

Comment 11

[vsireesha246]

Along with the Mai fix for V1.4 (https://bugzilla.mozilla.org/show_bug.cgi?id=1034421#c9) the have the below queries.

1.When 'iccdetected' event is not fired in case of airplane mode is turn ON state to turn off state is changed.
2.In airplane mode case also i am getting cardState is 'ready' value.
3.What happens in IccManager interface if airplane mode ON/OFF case to ICCID?
4.How can we distinguish between airplane mode active state and 'No sim" case if airplane mode is active.

Comment 12

[Albert [:albert]]

Clearing ni because htsai can answer better than me.

Comment 13

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

(In reply to vsireesha246 from comment #11)
> Along with the Mai fix for V1.4
> (https://bugzilla.mozilla.org/show_bug.cgi?id=1034421#c9) the have the below
> queries.
> 

Please do not bind the airplane mode status with icc status since these two stand for different things. That said, for some hardware design, the power of icc remains on even if radio is off. In this case, the platform can detect the sim card. But there is still another hardware design, i.e. the power of icc is turned of along with the radio.

If app wants to know the existence of a sim card, IccManager or mobileConnection.iccId is the right API. If app wants to know the radio state, mobileConnection.radioState should be referred.

> 1.When 'iccdetected' event is not fired in case of airplane mode is turn ON
> state to turn off state is changed.

As I tried to explain above, 'iccdetected' is fired when the platform detects a new card, not because of radio being turned on. 

> 2.In airplane mode case also i am getting cardState is 'ready' value.

It's possible due to the vendor hardware design!

> 3.What happens in IccManager interface if airplane mode ON/OFF case to ICCID?

Same as Q1 ?

> 4.How can we distinguish between airplane mode active state and 'No sim"
> case if airplane mode is active.

Use |navigator.mobileConnections[x].iccId| or |navigator.iccManager| to know if a card is detected.
Use |navigator.mobileConnections[x].radioState| to know if airplane mode is on or off.

Hope it's clear!

Comment 14

[Marina Rodríguez [:mai]]

Hi,
we need to use the event iccdetected after the airplanemode is disabled because on some device after the airplanemodechangestate event is fired, the ril is too slowly awakening and the IccManager or mobileConnection apis are not ready if we access to then in that moment. 

We need ensure the icc exists before relaunched the costcontrol app, if we cannot use this event, how can detect the moment that both apis are ready?

Regards

Comment 15

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

(In reply to marina rodríguez [:mai] from comment #14)
> Hi,
> we need to use the event iccdetected after the airplanemode is disabled
> because on some device after the airplanemodechangestate event is fired, the
> ril is too slowly awakening and the IccManager or mobileConnection apis are
> not ready if we access to then in that moment. 
> 
> We need ensure the icc exists before relaunched the costcontrol app, if we
> cannot use this event, how can detect the moment that both apis are ready?
> 
> Regards

Already talked on IRC. Clearing NI!
Flag me if you still have questions :)

Comment 16

[Salvador de la Puente González [:salva]]

Comment on attachment 8452238 [details] [review]
Patch For Master branch

Ok, no blocking comments on GitHub. Thank you Mai!

Comment 17

[vsireesha246]

As per htsai comment#13 the behaviour of 'iccdetected' event firing from Gecko is only when ever the new ICC detected.

So the existing code is not working for all devices.
So we need to change the gaia costcontrol code in case of airplane mode state as device independent.

As per Mai IRC chat app need to listen for 'iccchange' event instead of 'iccdetected'.
But we need to consider Multi-SIM case also.

Comment 18

[vsireesha246]

Attachment: Bug_1034421_iccdetected.patch

Comment 19

[vsireesha246]

Mai would you please apply attached Bug_1034421_iccdetected.patch on top of your patch.

This patch will solve the problem of 'iccdetected' event firing is not happen case.
and also solves the race conditions if any.I just add few conditions,which will work without any side effects.
Please check this patch on DS devices and let me know the status.

Thanks..
Sireesha



(In reply to marina rodríguez [:mai] from comment #9)
> Created attachment 8452236 [details] [review]
> Patch for v1.4 to Sireesha
> 
> This patch combine a rebase for v1.4 of the patch 960500 + the patch for
> this bug. It's only for testing purposes

Comment 20

[Marina Rodríguez [:mai]]

Master: fe313d0dc6b51aafc8d971021535707f8155f8fc

Comment 22

[Marina Rodríguez [:mai]]

Comment on attachment 8452238 [details] [review]
Patch For Master branch

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.
The app didn't consider this scenario. Then when the airplanemode is disabled without simcard the message of airplanemode disabled is not changed to no simcard after the airplanemode is disabled
[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 960500
[User impact] if declined: Show the message airplanemode enabled when the airplaneme is disabled (only happens without simcard)
[Testing completed]: yes
[Risk to taking this patch] (and alternatives if risky): Low risk
[String changes made]:

Comment 23

[bhavana bajaj [:bajaj]]

(In reply to marina rodríguez [:mai] from comment #22)
> Comment on attachment 8452238 [details] [review]
> Patch For Master branch
> 
> NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to
> better understand the B2G approval process and landings.
> The app didn't consider this scenario. Then when the airplanemode is
> disabled without simcard the message of airplanemode disabled is not changed
> to no simcard after the airplanemode is disabled
> [Approval Request Comment]
> [Bug caused by] (feature/regressing bug #): 960500
> [User impact] if declined: Show the message airplanemode enabled when the
> airplaneme is disabled (only happens without simcard)
> [Testing completed]: yes
> [Risk to taking this patch] (and alternatives if risky): Low risk
> [String changes made]:

Marking this as a regression as this is a fallout from 960500 and instead making this a blocker.

Comment 24

[bhavana bajaj [:bajaj]]

Comment on attachment 8452238 [details] [review]
Patch For Master branch

Clearing the approval request here as the blocking 2.0+ should take care of uplifting this.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/5a3b33de5adee0bcb59ef92f6e3d67741636f99f