1009134 - Transition away from using dataset.placeholder to localize placeholder

Status: RESOLVED FIXED

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

Description

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

Spin-off from bug 994290.

SMS is currently using a quirky feature of l10n.js to localize two input's placeholders.

We want to clean up node attribute localization and remove those quirks.

The code we're talking about is here:
 * https://github.com/mozilla-b2g/gaia/tree/e4d250f043e8d9334e3094103a46ccf1811a6114/apps/sms/locales/sms.en-US.properties#L43-L44
 * https://github.com/mozilla-b2g/gaia/blob/e4d250f043e8d9334e3094103a46ccf1811a6114/apps/sms/style/composer.css#L98-L104

I'm not sure if CSS will auto-update the value on language change, but I assume it will not.

If that's true, than moving away from dataset/CSS toward JS will also fix that.

Comment 1

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

CC'ing the author of the code.

Borja, would you mind transitioning away from this toward JS solution?

Comment 2

[Julien Wajsberg [:julienw]]

(taking the NI because Borja merely moved this code)

> I'm not sure if CSS will auto-update the value on language change, but I assume it will not.

I'm pretty sure it does.


How do you propose to fix this in JS? Adding a "ready" handler?
And in that "ready" handler we'd do a querySelector on [data-l10n-id] like we already do for our time headers?

Comment 3

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

Attachment: pull request

Yes!

Here comes the patch :)

Comment 4

[Julien Wajsberg [:julienw]]

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

One question about making the code more generic, but otherwise it's a no brainer.

Keeping the r request for now.

Comment 5

[Julien Wajsberg [:julienw]]

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

Removing the review request since we agreed on the path forward.
Please request review again once you're ready :)

Comment 6

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

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

Updated patch with js-l10n-placeholder and id->l10nId translation

Comment 7

[Julien Wajsberg [:julienw]]

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

r=me if you use Utils.camelCase instead of your own toCamelCase function.

I'm filing a bug for adding unit tests for this file (I understand you don't want to add unit tests for a file that's untested yet).

Comment 8

[Julien Wajsberg [:julienw]]

Bug 1012670 was created to track adding unit tests for startup.js

Comment 9

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

Switched to Utils.camelCase.

Patch: https://github.com/mozilla-b2g/gaia/commit/74635f4e507d00910cb1c11ffaf515fca112e6fb
Merge: https://github.com/mozilla-b2g/gaia/commit/3425e695233ff21a3054d088b5bef910f9751f12