1058795 - Carrier Info extended records display broken

Status: RESOLVED DUPLICATE of bug 1068860

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

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

While fixing the remote debugging prompt in bug 1058745, I noticed that the "extended records" part of CarrierInfoNotifier is likely broken due to the changes in bug 1029944.

In bug 1029944, ModalDialog was changed to take in a l10n ID, but CarrierInfoNotifier expects to pass generated text[1] for extended records.  It's not clear to me how that should work with the API change.

[1]: https://github.com/mozilla-b2g/gaia/blob/1c6bc15bfcc49a72658e2c841c214b9e5ad540fb/apps/system/js/carrier_info_notifier.js#L21

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Gandalf, how should this be repaired now that ModalDialog only allows l10n IDs?

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

If the API has to handle plain text that is not supposed to be localizable as well as localization, we recommend it to be handling it on the lowest possible level.

In this case, I would extend the ModalDialog to accept message as an object (with {id: l10nId, args: l10nArgs}) or plain text and then inside it do:

if (typeof message == 'string') {
  node.removeAttribute('data-l10n-id');
  node.textContent = message;
} else {
  navigator.mozL10n.setAttributes(node, message.id, message.args);
}

Does it sound like a possible solution in this case?

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

It seems okay, but I don't know much about this CarrierInfoNotifier and it's needs, as I just happened to notice the issue while searching around.

Alive, maybe you know who works on CarrierInfoNotifier and might need this fixed?

Comment 4

[(inactive after 6/18) Alive Kuo [:alive]]

git blame tell me rex is the last one changed the line but I am not sure if he is able to work on.

Comment 5

[KM Lee [:rexboy] (inactivate after Sep. 22)]

IIRC this patch is nothing but popping out messages sent from carrier.
No other interactions required.
This is made for matching the spec, but we are not sure if someone is really using it. :-/
I think it's a minor feature.

I can help modifying the patch by comment 2 if required.

Comment 6

[Jason Smith [:jsmith]]

Can you provide example STR for this bug? That will help QA in case we need to do a window on the bug.

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

I am not too sure, as I only noticed the issue by searching the codebase.  Perhaps :rexboy can help.

Comment 8

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Ken, do we have any real test cases on this bug?

Comment 9

[Ken Chang[:kenkai]]

(In reply to KM Lee [:rexboy][:蠟蠟蠟蠟] from comment #8)
> Ken, do we have any real test cases on this bug?
We used test code to test this. please see bug 882985.
This request was coming from partner. NI? partner.
Hi Anshul, do you have any suggest to test this?

Comment 10

[Anshul]

This feature is part of CDMA spec and is to handle different type of information record sent by the network, displaying some information on the UI is one of them. 

Ken, as part of bug 882985 you guys added a test script, is that not good enough to test for now? If not, even if I provide a test case to you, there is no easy way to simulate that on your end without the network simulators.

Comment 11

[KM Lee [:rexboy] (inactivate after Sep. 22)]

By Comment #10 from Anshul,
seems it's not very easy to do real testing without carrier support.

STR of bug 882985 is:
1. Carrier sends a CDMA carrier info (which is not easy to simulate)
2. System shows an alert dialog containing the info sent.

If the code based on the unit test works well on real environment in the past, all I can come up is that we just keep tracking the unit-test for this case.
We may add a test case examing the l10n issue above.

Jason, would this works for QAs?

Comment 12

[Jason Smith [:jsmith]]

Okay - sounds like we won't be able to test this on our end then if this is carrier-specific.

Comment 13

[:gerard-majax]

Everything points to this being the same as bug 1068860.

Comment 14

[yifan [:yifan][:yliao]]

(In reply to Zibi Braniecki [:gandalf] from comment #2)
> If the API has to handle plain text that is not supposed to be localizable
> as well as localization, we recommend it to be handling it on the lowest
> possible level.
> 
> In this case, I would extend the ModalDialog to accept message as an object
> (with {id: l10nId, args: l10nArgs}) or plain text and then inside it do:
> 
> if (typeof message == 'string') {
>   node.removeAttribute('data-l10n-id');
>   node.textContent = message;
> } else {
>   navigator.mozL10n.setAttributes(node, message.id, message.args);
> }
> 
> Does it sound like a possible solution in this case?

[e-mail] mentioned the fix would require changes of all use of ModalDialog.

How about using navigator.mozL10n.get to check if the message is a l10n id?

if (navigator.mozL10n.get(message)) {
  navigator.mozL10n.setAttributes(node, message);
} else {
  node.removeAttribute('data-l10n-id');
  // may need to escape message here
  node.textContent = message;
}