1457948 - Migrate in-content/privacy.js to Fluent

Status: RESOLVED FIXED

(Firefox :: Settings UI, enhancement, P3)

Description

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

There's a nice subset of strings in in-content/privacy.js that can be still migrated to Fluent without being blocked by anything.

Most of the are actually handled over to subdialogs with exceptions.

Comment 1

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

Attachment: Bug 1457948 - Migrate in-content/privacy.js to Fluent.

Review commit: https://reviewboard.mozilla.org/r/241820/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/241820/

Comment 2

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

Comment on attachment 8973357 [details]
Bug 1457948 - Migrate in-content/privacy.js to Fluent.

This is likely to be the first patch to land after 62 opens.

Flod - general feedback? Naming? Would you want to take the migration script?

Comment 3

[Francesco Lodolo [:flod]]

Comment on attachment 8973357 [details]
Bug 1457948 - Migrate in-content/privacy.js to Fluent.

https://reviewboard.mozilla.org/r/241820/#review247730

::: browser/locales/en-US/browser/preferences/permissions.ftl:70
(Diff revision 1)
> +## Exceptions - Cookies
> +
> +permissions-exceptions-cookie-window =
> +    .title = Exceptions - Cookies and Site Data
> +    .style = { permissions-window.style }
> +permissions-exceptions-cookie-desc = You can specify which websites are always or never allowed to use cookies and site data.  Type the exact address of the site you want to manage and then click Block, Allow for Session, or Allow.

Not sure if it makes sense to do it in this case, since we are migrating existing strings, but should we actually reference the button labels in these cases?

For sure, it would improve consistency, with the added risk of showing a confusing sentence if the button label is not translated

```
…to manage then click { permissions-block.label }, { permissions-session.label }, or { permissions-allow.label }
```

::: browser/locales/en-US/browser/preferences/permissions.ftl:72
(Diff revision 1)
> +permissions-exceptions-cookie-window =
> +    .title = Exceptions - Cookies and Site Data
> +    .style = { permissions-window.style }
> +permissions-exceptions-cookie-desc = You can specify which websites are always or never allowed to use cookies and site data.  Type the exact address of the site you want to manage and then click Block, Allow for Session, or Allow.
> +
> +## Exceptions - Popups

nit: Pop-ups

::: browser/locales/en-US/browser/preferences/permissions.ftl:86
(Diff revision 1)
> +permissions-exceptions-saved-logins-window =
> +    .title = Exceptions - Saved Logins
> +    .style = { permissions-window.style }
> +permissions-exceptions-saved-logins-desc = Logins for the following websites will not be saved
> +
> +## Exceptions - Addons

nit: Add-ons

::: browser/locales/en-US/browser/preferences/permissions.ftl:96
(Diff revision 1)
> +permissions-exceptions-addons-desc = You can specify which websites are allowed to install add-ons. Type the exact address of the site you want to allow and then click Allow.
> +
> +## Site Permissions - Notifications
> +
> +permissions-site-notification-window =
> +    .title = Settings - Notification Permissions 

nit: trailing whitespace

::: browser/locales/en-US/browser/preferences/permissions.ftl:120
(Diff revision 1)
> +permissions-site-camera-window =
> +    .title = Settings - Camera Permissions
> +    .style = { permissions-window.style }
> +permissions-site-camera-desc = The following websites have requested to access your camera. You can specify which websites are allowed to access your camera. You can also block new requests asking to access your camera.
> +permissions-site-camera-disable-label =
> +    .label = Block new requests asking to access your camera<Paste>

extra <paste>

Comment 4

[Francesco Lodolo [:flod]]

Comment on attachment 8973357 [details]
Bug 1457948 - Migrate in-content/privacy.js to Fluent.

File looks good, an open question about using references to button labels.

One additional risk that I forgot: if someone changes or removes a button, we'd need to update (and invalidate) all strings referencing it.

Comment 5

[Francesco Lodolo [:flod]]

Attachment: bug_1457948_incontent_privacy_js.py

Migration, pending question. If we need to replace text, we need the old format for those strings, since transforms_from() only supports COPY.

Comment 6

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

I don't have a strong opinion either way. Axel? Do you have a preference?

Comment 7

[Axel Hecht [:Pike]]

If anybody removes a button, we need to do that anyway (invalidate the current string).

I'd bet that we're inconsistent right now, so I wouldn't bet much on being able to search and replace and stuff. If we wanted to do that, we should just add a new non-migrated string.

I'd just go for what we have right now.

Comment 8

[Francesco Lodolo [:flod]]

(In reply to Axel Hecht [:Pike] from comment #7)
> If anybody removes a button, we need to do that anyway (invalidate the
> current string).

Yes

> I'd bet that we're inconsistent right now, so I wouldn't bet much on being
> able to search and replace and stuff.

That sounds likely.
 
> I'd just go for what we have right now.

Agreed.

Comment 9

[Francesco Lodolo [:flod]]

(In reply to Francesco Lodolo [:flod] from comment #5)
> Created attachment 8973425 [details]
> bug_1457948_incontent_privacy_js.py
> 
> Migration, pending question. If we need to replace text, we need the old
> format for those strings, since transforms_from() only supports COPY.

Note: we might be able to drastically improve this recipe's readability with bug 1459619, getting rid of the repeated path.

Comment 10

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

Comment on attachment 8973357 [details]
Bug 1457948 - Migrate in-content/privacy.js to Fluent.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/241820/diff/1-2/

Comment 11

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

Jared: this is a simple cleanup of the siteperms/exceptiosn subdialogs on top of the migration. I think Fluent makes it cleaner as we stop passing the strings around as argument to new window :)

I'll take the migration review since Flod wrote it.

Comment 12

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

Comment on attachment 8973357 [details]
Bug 1457948 - Migrate in-content/privacy.js to Fluent.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/241820/diff/2-3/

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8973357 [details]
Bug 1457948 - Migrate in-content/privacy.js to Fluent.

https://reviewboard.mozilla.org/r/241820/#review247972

Comment 14

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

Comment on attachment 8973357 [details]
Bug 1457948 - Migrate in-content/privacy.js to Fluent.

https://reviewboard.mozilla.org/r/241820/#review247988

the migration works as expected. Thank you flod!

Comment 15

[Mozilla Autoland]

hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 8 changes to 8 files
remote: (ftl_check check enabled per config override)
remote: 
remote: ************************ ERROR *************************
remote: You are trying to commit a change to an FTL file.
remote: At the moment modifying FTL files requires a review from
remote: one of the L10n Drivers.
remote: Please, request review from either:
remote:     - Francesco Lodolo (:flod)
remote:     - Zibi Braniecki (:gandalf)
remote:     - Axel Hecht (:pike)
remote:     - Stas Malolepszy (:stas)
remote: ********************************************************
remote: 
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.mozhooks hook failed
abort: push failed on remote

Comment 16

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

Comment on attachment 8973357 [details]
Bug 1457948 - Migrate in-content/privacy.js to Fluent.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/241820/diff/3-4/

Comment 17

[Francesco Lodolo [:flod]]

Comment on attachment 8973357 [details]
Bug 1457948 - Migrate in-content/privacy.js to Fluent.

https://reviewboard.mozilla.org/r/241820/#review248122

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fba5e06d6365
Migrate in-content/privacy.js to Fluent. r=flod,jaws

Comment 19

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

Comment on attachment 8973357 [details]
Bug 1457948 - Migrate in-content/privacy.js to Fluent.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/241820/diff/4-5/

Comment 20

[Dorel Luca [:dluca]]

Backed out changeset fba5e06d6365 (bug 1457948) for Browser-chrome failures on multipe files. CLOSED TREE 

Log:
https://treeherder.mozilla.org/logviewer.html#?job_id=177401691&repo=autoland&lineNumber=2244

INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_permissions_dialog.js | Number of permission items is 0 initially - 0 == 0 - 
[task 2018-05-08T07:40:54.222Z] 07:40:54     INFO - Buffered messages finished
[task 2018-05-08T07:40:54.223Z] 07:40:54     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_permissions_dialog.js | uncaught exception - TypeError: this._bundle is null at _getCapabilityString@chrome://browser/content/preferences/sitePermissions.js:168:5
[task 2018-05-08T07:40:54.226Z] 07:40:54     INFO - _addPermissionToList@chrome://browser/content/preferences/sitePermissions.js:175:28
[task 2018-05-08T07:40:54.227Z] 07:40:54     INFO - observe@chrome://browser/content/preferences/sitePermissions.js:135:7
[task 2018-05-08T07:40:54.228Z] 07:40:54     INFO - set@resource:///modules/SitePermissions.jsm:446:7
[task 2018-05-08T07:40:54.229Z] 07:40:54     INFO - addPermission@chrome://mochitests/content/browser/browser/components/preferences/in-content/tests/browser_permissions_dialog.js:51:3
[task 2018-05-08T07:40:54.230Z] 07:40:54     INFO - Async*Tester_execTest/<@chrome://mochikit/content/browser-test.js:1083:34
[task 2018-05-08T07:40:54.231Z] 07:40:54     INFO - async*Tester_execTest@chrome://mochikit/content/browser-test.js:1074:16
[task 2018-05-08T07:40:54.232Z] 07:40:54     INFO - nextTest/<@chrome://mochikit/content/browser-test.js:976:9
[task 2018-05-08T07:40:54.233Z] 07:40:54     INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59
[task 2018-05-08T07:40:54.234Z] 07:40:54     INFO - 
[task 2018-05-08T07:40:54.236Z] 07:40:54     INFO - Stack trace:
[task 2018-05-08T07:40:54.237Z] 07:40:54     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:simpletestOnerror:1655
[task 2018-05-08T07:40:54.237Z] 07:40:54     INFO - resource:///modules/SitePermissions.jsm:set:446
[task 2018-05-08T07:40:54.239Z] 07:40:54     INFO - chrome://mochitests/content/browser/browser/components/preferences/in-content/tests/browser_permissions_dialog.js:addPermission:51
[task 2018-05-08T07:40:54.242Z] 07:40:54     INFO - GECKO(1292) | JavaScript error: chrome://browser/content/preferences/sitePermissions.js, line 168: TypeError: this._bundle is null
[task 2018-05-08T07:40:54.242Z] 07:40:54     INFO - Not taking screenshot here: see the one that was previously logged
[task 2018-05-08T07:40:54.243Z] 07:40:54     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_permissions_dialog.js | 0 == 1 - JS frame :: chrome://mochitests/content/browser/browser/components/preferences/in-content/tests/browser_permissions_dialog.js :: addPermission :: line 54
[task 2018-05-08T07:40:54.243Z] 07:40:54     INFO - Stack trace:
[task 2018-05-08T07:40:54.244Z] 07:40:54     INFO - chrome://mochitests/content/browser/browser/components/preferences/in-content/tests/browser_permissions_dialog.js:addPermission:54
[task 2018-05-08T07:40:54.244Z] 07:40:54     INFO - Not taking screenshot here: see the one that was previously logged
[task 2018-05-08T07:40:54.245Z] 07:40:54     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_permissions_dialog.js | Uncaught exception - at chrome://mochitests/content/browser/browser/components/preferences/in-content/tests/browser_permissions_dialog.js:19 - TypeError: label is undefined
[task 2018-05-08T07:40:54.245Z] 07:40:54     INFO - Stack trace:
[task 2018-05-08T07:40:54.246Z] 07:40:54     INFO - checkPermissionItem@chrome://mochitests/content/browser/browser/components/preferences/in-content/tests/browser_permissions_dialog.js:19:3
[task 2018-05-08T07:40:54.247Z] 07:40:54     INFO - addPermission@chrome://mochitests/content/browser/browser/components/preferences/in-content/tests/browser_permissions_dialog.js:55:3
[task 2018-05-08T07:40:54.248Z] 07:40:54     INFO - Async*Tester_execTest/<@chrome://mochikit/content/browser-test.js:1083:34
[task 2018-05-08T07:40:54.249Z] 07:40:54     INFO - async*Tester_execTest@chrome://mochikit/content/browser-test.js:1074:16
[task 2018-05-08T07:40:54.249Z] 07:40:54     INFO - nextTest/<@chrome://mochikit/content/browser-test.js:976:9
[task 2018-05-08T07:40:54.250Z] 07:40:54     INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59
[task 2018-05-08T07:40:54.250Z] 07:40:54     INFO - Leaving test bound addPermission
[task 2018-05-08T07:40:54.253Z] 07:40:54     INFO - Entering test bound observePermissionChange
[task 2018-05-08T07:40:54.254Z] 07:40:54     INFO - Not taking screenshot here: see the one that was previously logged
[task 2018-05-08T07:40:54.255Z] 07:40:54     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_permissions_dialog.js | uncaught exception - TypeError: p is undefined at observe@chrome://browser/content/preferences/sitePermissions.js:139:7
[task 2018-05-08T07:40:54.256Z] 07:40:54     INFO - set@resource:///modules/SitePermissions.jsm:446:7
[task 2018-05-08T07:40:54.257Z] 07:40:54     INFO - observePermissionChange@chrome://mochitests/content/browser/browser/components/preferences/in-content/tests/browser_permissions_dialog.js:64:3
[task 2018-05-08T07:40:54.258Z] 07:40:54     INFO - Async*Tester_execTest/<@chrome://mochikit/content/browser-test.js:1083:34
[task 2018-05-08T07:40:54.258Z] 07:40:54     INFO - async*Tester_execTest@chrome://mochikit/content/browser-test.js:1074:16
[task 2018-05-08T07:40:54.259Z] 07:40:54     INFO - nextTest/<@chrome://mochikit/content/browser-test.js:976:9
[task 2018-05-08T07:40:54.261Z] 07:40:54     INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=fba5e06d6365e3a99479832e794c3a2d8382bdc4

Backout:
https://hg.mozilla.org/integration/autoland/rev/f1dc07fd72a40ebbaf9a414b62130067a3ac9bc3

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/00722b20b907
Migrate in-content/privacy.js to Fluent. r=flod,jaws

Comment 22

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/00722b20b907