1029944 - Switch ModalDialog and AppModalDialog from using strings to using l10nIds

Status: RESOLVED FIXED

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

Description

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

Currently both ModalDialog and AppModalDialog API's rely on passing strings that are injected into DOM.

This is overly complicated and limits the localizability of the dialogs.

Currently the code calls mozL10n.get(l10nId) and passes the value to ModalDialog's config yesText, noText and title which uses textContent or innerHTML to inject it.

Instead, the code should pass the l10nId and ModalDialog should just set node.setAttribute('data-l10n-id', l10nId).

This bug is part of an ongoing effort to increase the localizability of Firefox OS UI by moving as much l10n as possible to go through declarative l10nId/l10nArgs (bug 1020138).

Comment 1

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

I confirmed that no use cases rely on escapeHTML so it's mostly about this function: https://github.com/mozilla-b2g/gaia/blob/669bb17a487e53b2c62d628d0e1d1e94968da919/apps/system/js/modal_dialog.js#L156-L227

and all its use cases. We need to switch the title and message to be l10nId's or null and yesText and noText to be l10nId's as well.

It's fairly easy and I'll be happy to mentor if anyone wants to give it a try.

Comment 2

[yifan [:yifan][:yliao]]

Attachment: pull request

Replaced mozL10n.get(l10nId) calls with node.setAttribute('data-l10n-id', l10nId) in ModalDialog and its use cases.

Comment 3

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

Comment on attachment 8465993 [details] [review]
pull request

That looks good! Thanks a lot :)

Comment 4

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

It also reduces the amount of innerHTML's!

Comment 5

[yifan [:yifan][:yliao]]

Comment on attachment 8465993 [details] [review]
pull request

Thank you! I later found out the test failed.

Test code of ModalDialog is updated. Changed comparisons of innerHtml and textContent to 'data-l10-id' attribute. Please see if the change conflicts anything.

Comment 6

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

Comment on attachment 8465993 [details] [review]
pull request

r+!

Comment 7

[yifan [:yifan][:yliao]]

Thank you!

Merged in master:
https://github.com/mozilla-b2g/gaia/pull/22387

TBPL: 
https://tbpl.mozilla.org/?rev=6205a92411686ae41a8343462ce5264a34c0423e&tree=Gaia-Try

Comment 8

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

Can we close this bug now?

Comment 9

[yifan [:yifan][:yliao]]

Yes, forgot that, sorry about it.