Closed Bug 1457948 Opened 6 years ago Closed 6 years ago

Migrate in-content/privacy.js to Fluent

Categories

(Firefox :: Settings UI, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 62
Tracking Status
firefox62 --- fixed

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Priority: -- → P3
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?
Attachment #8973357 - Flags: feedback?(francesco.lodolo)
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 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.
Attachment #8973357 - Flags: feedback?(francesco.lodolo) → feedback+
Attached file bug_1457948_incontent_privacy_js.py (obsolete) —
Migration, pending question. If we need to replace text, we need the old format for those strings, since transforms_from() only supports COPY.
Attachment #8973425 - Attachment mime type: text/x-python-script → text/plain
I don't have a strong opinion either way. Axel? Do you have a preference?
Flags: needinfo?(l10n)
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.
Flags: needinfo?(l10n)
(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.
(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.
Attachment #8973425 - Attachment is obsolete: true
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 on attachment 8973357 [details]
Bug 1457948 - Migrate in-content/privacy.js to Fluent.

https://reviewboard.mozilla.org/r/241820/#review247972
Attachment #8973357 - Flags: review?(jaws) → review+
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!
Attachment #8973357 - Flags: review?(gandalf) → review+
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 on attachment 8973357 [details]
Bug 1457948 - Migrate in-content/privacy.js to Fluent.

https://reviewboard.mozilla.org/r/241820/#review248122
Attachment #8973357 - Flags: review?(francesco.lodolo) → review+
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fba5e06d6365
Migrate in-content/privacy.js to Fluent. r=flod,jaws
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
Flags: needinfo?(gandalf)
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/00722b20b907
Migrate in-content/privacy.js to Fluent. r=flod,jaws
Flags: needinfo?(gandalf)
https://hg.mozilla.org/mozilla-central/rev/00722b20b907
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Depends on: 1462675
You need to log in before you can comment on or make changes to this bug.