1163162 - GCF Test case 31.8.7 failure: Call barring notification is not displayed

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Dialer, defect)

Description

[pgravel]

From 3GPP TS 51.010 section 31.8.7

Scenario:
1. Initiate an MO call.
2. Network does a remote hangup with cause “Operator determined Barring”.
3. UE is expected to display the call barring notification on UI but it display a generic "You cannot make a phone call at this time" error message.

Comment 1

[pgravel]

side note: I do not believe this is a regression, but still affect conformance testing.

Comment 2

[Johan Lorenzo [:jlorenzo]]

Comms triage: Certification blocker.

Comment 3

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

ni Gabriele as a soft reminder

Comment 4

[Gabriele Svelto [:gsvelto] (PTO)]

Thanks for the reminder, I hadn't noticed this became a blocker. It looks like this condition should correspond to the RIL OperatorDeterminedBarringError error, I'll try to cook up a patch to address this. Flagging as late-l10n since I'll need to add a string and this is *horribly late*.

Comment 5

[GitHub Autolander Bot]

Attachment: [gaia] gabrielesvelto:bug-1163162-operator-barring-error > mozilla-b2g:master

Comment 6

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8604725 [details] [review]
[gaia] gabrielesvelto:bug-1163162-operator-barring-error > mozilla-b2g:master

I'm not super-sure what text to use in this case so I went for "Call was barred by the operator" for the body with the pre-existing "Unable to make a phone call now" title. I hope this is appropriate. To the reporter, since I cannot check if this fixes the issue can you test it for me?

Comment 7

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8604725 [details] [review]
[gaia] gabrielesvelto:bug-1163162-operator-barring-error > mozilla-b2g:master

With Bhavana gone, I'm not sure who to ask about late-l10n, as I don't think that we're accepting new strings anymore for v2.2. :flod, do you know?

If we can't accept a new string, we could re-use 'unableToCallTitle' and 'unableToCallMessage' for the v2.2 uplift, and use this patch as-is for master.

I'm r+'ing this for master only. We need more info before we uplift it.

Comment 8

[Francesco Lodolo [:flod]]

According to b2g-internal, Josh Cheng is acting as release driver for 2.2, so he should be the one to give approval for 2.2

The extended string freeze ended on April 29, so I would be against having new strings on 2.2, but it's also not my decision.

String reusing: it would be safer to use unableToCallMessage, the other one is a title and might get some unexpected results.

Comment 9

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8604725 [details] [review]
[gaia] gabrielesvelto:bug-1163162-operator-barring-error > mozilla-b2g:master

(In reply to Francesco Lodolo [:flod] (UTC+2) from comment #8)

Thanks for the info. I'm preemptively setting the approval flag accordingly.

Comment 10

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8604725 [details] [review]
[gaia] gabrielesvelto:bug-1163162-operator-barring-error > mozilla-b2g:master

(In reply to Francesco Lodolo [:flod] (UTC+2) from comment #8)
> String reusing: it would be safer to use unableToCallMessage, the other one
> is a title and might get some unexpected results.

Oh, I missed this and thought you were referring to just v2.2 at first. Should we just add a new string for the title instead? That seems like a better idea to me.

Comment 11

[pgravel]

Patch works for me, thanks Gabriele.

Comment 12

[Francesco Lodolo [:flod]]

(In reply to Doug Sherk (:drs) (use needinfo?) from comment #10)
> Oh, I missed this and thought you were referring to just v2.2 at first.
> Should we just add a new string for the title instead? That seems like a
> better idea to me.

My mistake, I only looked at the string added on master and your question, not at the whole code. I thought you were asking to use unableToCallTitle as a message on 2.2.

The patch for master is fine as it is. You're creating a dialog with unableToCallTitle+callBarredByTheOperatorMessage, unableToCallTitle is reused in the same context (title) so it should be fine. If you want to create a new string ad-hoc for this dialog, that would work as well.

On 2.2 we should reuse existing string instead, like unableToCallTitle+unableToCallMessage

Comment 13

[Doug Sherk (:drs) (inactive)]

(In reply to Francesco Lodolo [:flod] (UTC+2) from comment #12)

Thanks, sounds good.

Comment 14

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Francesco Lodolo [:flod] (UTC+2) from comment #12)
> On 2.2 we should reuse existing string instead, like
> unableToCallTitle+unableToCallMessage

The problem here is that we already use that couple on v2.2 so without the new string the behavior on v2.2 is unchanged and the bug unfixed.

Comment 15

[Francesco Lodolo [:flod]]

(In reply to Gabriele Svelto [:gsvelto] from comment #14)
> The problem here is that we already use that couple on v2.2 so without the
> new string the behavior on v2.2 is unchanged and the bug unfixed.

For 2.2 it would be a WONTFIX. But, again, not the person who makes the call, that's up to release drivers.

Comment 16

[Keven Kuo [:kkuo]]

Hi! Josh,

Due to String Freeze for 2.2 on Apr 6th, is there anything we can do for new adding string?
Thanks

--
Keven

Comment 17

[Sean Lee [:seanlee][:weilonge]]

Attachment: Force using human-readable string for special case.

Based on attachment 8604725 [details] [review], this patch is an alternative way if we can not change l10n strings. Just for your reference.

Comment 18

[Gabriele Svelto [:gsvelto] (PTO)]

OK, landing on master then we'll figure out what to do for v2.2+. I can see why attachment 8605090 [details] [diff] [review] is an obvious solution but I don't think it's really useful for our partners since the string cannot be localized. Arguably an incomplete message in your language (as we currently have) is better than a more complete message in a language you don't understand.

Comment 19

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/c1d299f3d2f964b3d71c0cd7f81b0560e2313e33

Comment 20

[Josh Cheng [:josh]]

(In reply to Keven Kuo [:kkuo] from comment #16)
> Hi! Josh,
> 
> Due to String Freeze for 2.2 on Apr 6th, is there anything we can do for new
> adding string?
> Thanks
> 
> --
> Keven

Hi Keven,

I think we need to find a workaround or wontix in 2.2 given:

* we're way past FC and string freeze
* This wasn't a blocker in previous releases.
* We already passed l10n testing cycle.
* as Flod mentions, I'm also against breaking string freeze - especially so far in the 2.2 cycle.

Thanks.

Comment 21

[Sean Lee [:seanlee][:weilonge]]

Attachment: trial patch

Hi Gabriele,

Could you help to see the patch that reusing the string "unableToCallMessage"?
Thank you.

Comment 22

[Francesco Lodolo [:flod]]

(In reply to Sean Lee [:seanlee] from comment #21)
> Could you help to see the patch that reusing the string
> "unableToCallMessage"?

Isn't that what 2.2 is already doing?

Comment 23

[Sean Lee [:seanlee][:weilonge]]

Hi Francesco,

In attachment 8604725 [details] [review], 'OperatorDeterminedBarringError' is a new case should be handled with the message:
+    case 'OperatorDeterminedBarring':
+      dialogTitle = 'unableToCallTitle';
+      dialogBody = 'callBarredByTheOperatorMessage';
+      break;

The solution in attachment 8605701 [details] [diff] [review] is to re-use 'unableToCallMessage' for dialogBody.
I just checked the message again. I am not sure if the meaning of 'callBarredByTheOperatorMessage' can be replaced with 'unableToCallMessage'. That's why I ni Gabriele. 
Thank you.

Comment 24

[Gabriele Svelto [:gsvelto] (PTO)]

We're already displaying the unableToCallMessage body in v2.2 so attachment 8605701 [details] [diff] [review] is not needed for having that behavior. The issue in comment 0 is precisely that the message is considered too vague. If we cannot land new strings on v2.2 then there's no way to fix this (hard-coding an English string in the code sounds like a bad solution to me) and we should WONTFIX and remove the 2.2+ flag.

Comment 25

[Sean Lee [:seanlee][:weilonge]]

Hi Gabriele, Thanks a lot for your explanation.

Comment 26

[Anshul]

Seems like its too late for 2.2. I am removing the 2.2 dependency on this bug. Please continue to fix this on master though.