Closed Bug 1147160 Opened 5 years ago Closed 5 years ago

[Network Alert] Presidential alerts not displayed when Emergency Alert is off for GSM

Categories

(Firefox OS Graveyard :: Gaia::Network Alerts, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.2+, b2g-v2.2 fixed, b2g-master fixed)

RESOLVED FIXED
2.2 S9 (3apr)
blocking-b2g 2.2+
Tracking Status
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: cyang, Assigned: julienw)

References

Details

(Whiteboard: [caf priority: p2][CR 813152])

Attachments

(1 file)

While on GSM network, when a Presidential alert has been received, the UI does not display this message when Emergency Alert is toggled off.

STR:

1) Go to Settings->Messaging Settings
2) Toggle 'Emergency Alert' to off
3) Inject a Presidential alert cell broadcast message (message id 4370)

Actual: Nothing is displayed on the screen
Expected: The Presidential alert is displayed on the screen

Presidential Level Alerts are not settable by MMI and should always be displayed regardless of the UI settings. Reference TS at [1], table under section 9.4.1.2.2 for 'Not settable by MMI'.

Note that this bug is similar to bug 1091960 but that bug was more specific to incorrect value checks for CDMA support. Opening a new bug as this is only occurring for GSM.

The reason for this bug is because at [2], cell_broadcast_system.js returns early to let Network Alert app handle CMAS messages. However, the check at [3] prevents the Presidential alert from being displayed.


[1] http://www.etsi.org/deliver/etsi_ts/123000_123099/123041/11.06.00_60/ts_123041v110600p.pdf
[2] https://github.com/mozilla-b2g/gaia/blob/v2.2/apps/system/js/cell_broadcast_system.js#L80
[3] https://github.com/mozilla-b2g/gaia/blob/v2.1/apps/network-alerts/js/index.js#L37
Thanks Carol, this is indeed a better idea to file a separate bug.

I definitely agree we should fix this.

Bevis, do you know how we can know the alert is a Presidential alert from Gaia?
blocking-b2g: --- → 2.2?
Flags: needinfo?(btseng)
(In reply to Julien Wajsberg [:julienw] from comment #1)
> Thanks Carol, this is indeed a better idea to file a separate bug.
> 
> I definitely agree we should fix this.
> 
> Bevis, do you know how we can know the alert is a Presidential alert from
> Gaia?

The quick fix would be at [3], check for message.messageId for Presidential alert id 4370
From what I read this requirement could be different depending on the carrier. Maybe we should make it depend on the mcc/mnc ?
> The quick fix would be at [3], check for message.messageId for Presidential alert id 4370

Actually on second thought, the check at [3] should just be removed altogether. The filtering of messages would have already been done down at the RIL level. UI should not have to do such filtering. For example: 

 ril.cellbroadcast.searchlist = ""
 ril.cellbroadcast.disabled = false
 cmas.enabled = false

Then the only CB messages that would be received by the UI is Presidential Alert only. It would seem that the check in Gaia is redundant to what the lower layers are already doing.
(In reply to Carol Yang [:cyang] from comment #4)
> > The quick fix would be at [3], check for message.messageId for Presidential alert id 4370
> 
> Actually on second thought, the check at [3] should just be removed
> altogether. The filtering of messages would have already been done down at
> the RIL level. UI should not have to do such filtering. For example: 
> 
>  ril.cellbroadcast.searchlist = ""
>  ril.cellbroadcast.disabled = false
>  cmas.enabled = false
> 
> Then the only CB messages that would be received by the UI is Presidential
> Alert only. It would seem that the check in Gaia is redundant to what the
> lower layers are already doing.

Due to these 2 reasons:
1. "cmas.enabled" is a UI settings for user to choose whether Emergency Alert shall be displayed or not.
2. Presidential-Level Alert is "Not settable by MMI".

The solution is to identify "Presidential-Level" Alert and then, display it without checking 'cmas.enabled'.
In GSM network, 'messageId' is used to identify the Presidential-Level CMAS message if its value is equal to "4370" or "4383". Reference TS at [1], table under section 9.4.1.2.2.

[1] http://www.etsi.org/deliver/etsi_ts/123000_123099/123041/11.06.00_60/ts_123041v110600p.pdf
Flags: needinfo?(btseng)
Bevis, can you also please comment on comment 4? Is this check done in Gecko already?
Flags: needinfo?(btseng)
IMO cmas.enabled should be checked by UI because it's purely a UI concern.
(In reply to Julien Wajsberg [:julienw] from comment #6)
> Bevis, can you also please comment on comment 4? Is this check done in Gecko
> already?

I don't think 'ril.cellbroadcast.searchlist = ""' is a check but actually triggers modem to clear the search list settable from MMI but all the channels in CBMI/CBMIR/CBMID of the inserted UICC will still be applied according to MOZ-RIL's implementation.

In addition, 'ril.cellbroadcast.disabled = false' triggers modem to deactivate the CBS service.
However, according to the discussion in bug 100851, we still don't have too much information about how different vendor/modem solution behaves: 
Does all cbs messages are disabled or only the ones settable by MMI when deactivated?

That's why I provide a different suggestion in comment 5 to fix this bug.

We can NI Carol to double confirm if this proposal is better.
Flags: needinfo?(btseng)
Flags: needinfo?(cyang)
(In reply to Julien Wajsberg [:julienw] from comment #7)
> IMO cmas.enabled should be checked by UI because it's purely a UI concern.

I agree with you that this is purely UI concern.
The only concern is that if this UI design conflict to any certificate criteria especially to the Presidential-Level Alerts.
(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #9)
> (In reply to Julien Wajsberg [:julienw] from comment #7)
> > IMO cmas.enabled should be checked by UI because it's purely a UI concern.
> 
> I agree with you that this is purely UI concern.
> The only concern is that if this UI design conflict to any certificate
> criteria especially to the Presidential-Level Alerts.

Yes, then this is a bug and we need to fix it :)
(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #8)
> (In reply to Julien Wajsberg [:julienw] from comment #6)
> > Bevis, can you also please comment on comment 4? Is this check done in Gecko
> > already?
> 
> I don't think 'ril.cellbroadcast.searchlist = ""' is a check but actually
> triggers modem to clear the search list settable from MMI but all the
> channels in CBMI/CBMIR/CBMID of the inserted UICC will still be applied
> according to MOZ-RIL's implementation.
> 
> In addition, 'ril.cellbroadcast.disabled = false' triggers modem to
> deactivate the CBS service.
> However, according to the discussion in bug 100851, we still don't have too
> much information about how different vendor/modem solution behaves: 
> Does all cbs messages are disabled or only the ones settable by MMI when
> deactivated?
> 
> That's why I provide a different suggestion in comment 5 to fix this bug.
> 
> We can NI Carol to double confirm if this proposal is better.

Hi Bevis,

You're solution in comment 5 will fix the issue.

Thanks,
Carol
Flags: needinfo?(cyang)
Whiteboard: [CR 813152]
Whiteboard: [CR 813152] → [caf priority: p2][CR 813152]
this seems to be a new feature so I doubt if we should implement this at this moment.
is this cert blocker?
(In reply to Wesley Huang [:wesley_huang] (EPM) (NI me) from comment #12)
> this seems to be a new feature so I doubt if we should implement this at
> this moment.
> is this cert blocker?

Without a fix, this would be breaking the spec. Also, note that this works for the CDMA case.
This looks very easy to do, let's not spend too much time to discuss and let's just do it for v2.2. I can take on the work to do it.
Assignee: nobody → felash
blocking-b2g: 2.2? → 2.2+
Comment on attachment 8585497 [details] [review]
[gaia] julienw:1147160-presidential-alerts > mozilla-b2g:master

Hey Carol,

I don't have an easy way to check that myself yet (I asked for support from our emulator peers so that I can test using the emulator). Could you please test this patch on your side?

Thanks
Attachment #8585497 - Flags: feedback?(cyang)
Comment on attachment 8585497 [details] [review]
[gaia] julienw:1147160-presidential-alerts > mozilla-b2g:master

I see the Presidential Alert now
Attachment #8585497 - Flags: feedback?(cyang) → feedback+
Comment on attachment 8585497 [details] [review]
[gaia] julienw:1147160-presidential-alerts > mozilla-b2g:master

Thanks,

let's ask a review then !
Attachment #8585497 - Flags: review?(schung)
Comment on attachment 8585497 [details] [review]
[gaia] julienw:1147160-presidential-alerts > mozilla-b2g:master

Only some small nits but looks great overall, thanks!
Attachment #8585497 - Flags: review?(schung) → review+
Keywords: checkin-needed
https://github.com/mozilla-b2g/gaia/pull/29220

Autolander could not land the pull request due to not having collaborator rights. This is possibly due to a tree closure. Please check the tree status and request checkin again once the tree is open.
master: b7aafb22f3018091553a499b4b343c2340553ef9
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Comment on attachment 8585497 [details] [review]
[gaia] julienw:1147160-presidential-alerts > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): -
[User impact] if declined: Presidential alerts can be opt out.
[Testing completed]: yes, both manually and unit tests.
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: none
Attachment #8585497 - Flags: approval-gaia-v2.2?
Bhavana, please get this uplifted to 2.2.
Flags: needinfo?(bbajaj)
Flags: needinfo?(bbajaj)
Attachment #8585497 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
You need to log in before you can comment on or make changes to this bug.