1304306 - Need to have a place in the Preference -> Setting for users to launch the profile list add/edit/remove dialog

Status: RESOLVED FIXED

(Toolkit :: Form Manager, defect, P3)

Description

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

To have a place(maybe button) in the Preference -> Privacy for users to launch the profile list add/edit/remove dialog.

Comment 1

[Matthew N. [:MattN]]

We should be able to listen for the advanced-pane-loaded observer notification and used the subject `window` to know when to modify the about:preferences page: https://dxr.mozilla.org/mozilla-central/rev/7ae377917236b7e6111146aa9fb4c073c0efc7f4/browser/components/preferences/in-content/preferences.js#91

Comment 2

[Scott Wu [:scottwu]]

Attachment: Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

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

Comment 3

[Scott Wu [:scottwu]]

Here's what I got so far: https://reviewboard.mozilla.org/r/101504/diff/#index_header

It is possible to inject the form autofill preference in the privacy tab, but there are a few things I'm not sure about:
- I put the code in FormAutofillContent.js, and listen for the "advanced-pane-loaded" notification, but this code will only be used in a about:preferences page and no where else. Wonder if that's acceptable?
- I can't remove the observer when done because it will cause problem if the page is reloaded
- The <preference> XUL element doesn't sync well with the UI

Wonder if you think I'm heading in the right direction Matt? Just want to hear your thoughts before I go any further with this :)

Comment 4

[Matthew N. [:MattN]]

Comment on attachment 8822626 [details]
Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

https://reviewboard.mozilla.org/r/101504/#review104012

Here is my partial review…

::: browser/extensions/formautofill/content/FormAutofillContent.js:283
(Diff revision 1)
> +  formAutofillGroup.setAttribute("data-category", "panePrivacy");
> +  formAutofillGroup.setAttribute("hidden", !this.isPrivacyPane);
> +
> +  captionLabel.textContent = "Form Autofill";
> +
> +  checkboxEnable.setAttribute("label", "Enable Profile Autofill");

You'll need to put this string in a new .properties file eventually

::: browser/extensions/formautofill/content/FormAutofillContent.js:293
(Diff revision 1)
> +    checkboxEnable.setAttribute("checked", true);
> +  }
> +
> +  spacer.setAttribute("flex", 1);
> +
> +  btnSavedProfiles.setAttribute("label", "Saved Profiles...");

Same here about .properties.

::: browser/extensions/formautofill/content/FormAutofillContent.js:335
(Diff revision 1)
> +  }
> +};
> +
> +// Listens to "advanced-pane-loaded" from about:preferences, and inject the
> +// Form Autofill preferences on top of the location bar group in privacy tab.
> +Services.obs.addObserver(function insertAutofillPref(window) {

This observer should be removed when the extension is uninstalled to prevent leaking. It may also need to be removed at shutdown if it gets triggered as a leak in debug builds.

::: browser/extensions/formautofill/content/FormAutofillContent.js:338
(Diff revision 1)
> +// Listens to "advanced-pane-loaded" from about:preferences, and inject the
> +// Form Autofill preferences on top of the location bar group in privacy tab.
> +Services.obs.addObserver(function insertAutofillPref(window) {
> +  let document = window.document;
> +  // Checks if the form autofill preference already exists. This is necessary
> +  // because if the observer is removed when used, the listener wouldn't be

You aren't removing the observer when used though (which is fine).

::: browser/extensions/formautofill/content/FormAutofillContent.js:346
(Diff revision 1)
> +    let formAutofillPref = new FormAutofillPref(document);
> +    let parentNode = document.getElementById("mainPrefPane");
> +    let insertBeforeNode = document.getElementById("locationBarGroup");
> +    parentNode.insertBefore(formAutofillPref, insertBeforeNode);
> +  }
> +}, "advanced-pane-loaded", false);

Are you sure this works with e10s on since I thought about:preferences was loaded in the parent process.

Comment 5

[Scott Wu [:scottwu]]

Comment on attachment 8822626 [details]
Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

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

Comment 6

[Scott Wu [:scottwu]]

Comment on attachment 8822626 [details]
Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

https://reviewboard.mozilla.org/r/101504/#review104012

Thanks for the initial review Matt. You were right about about:preferences being loaded in parent process, and most of the code should be in parent jsm rather than in content. I've moved the code and added .properties file for localization.

Comment 7

[Matthew N. [:MattN]]

Comment on attachment 8822626 [details]
Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

https://reviewboard.mozilla.org/r/101504/#review106198

::: browser/extensions/formautofill/FormAutofillParent.jsm:45
(Diff revision 2)
>  
>  const PROFILE_JSON_FILE_NAME = "autofill-profiles.json";
>  
>  let FormAutofillParent = {
>    _profileStore: null,
> +  _formAutofillPref: null,

Can we avoid keeping a reference here? This doesn't work when there are multiple tabs open to about:preferences

::: browser/extensions/formautofill/FormAutofillParent.jsm:129
(Diff revision 2)
> +    let os = Cc["@mozilla.org/observer-service;1"]
> +               .getService(Ci.nsIObserverService);
> +    os.removeObserver(this, "advanced-pane-loaded", false);

Services.obs.removeObserver

::: browser/extensions/formautofill/FormAutofillParent.jsm:214
(Diff revision 2)
> +                  .getService(Ci.nsIStringBundleService)
> +                  .createBundle("chrome://formautofill/locale/formautofill.properties");
> +
> +  this.prefService = Cc["@mozilla.org/preferences-service;1"]
> +                       .getService(Ci.nsIPrefService)
> +                       .getBranch("browser.formautofill.");

Using subset prefBranches other than for observing is generally frowned upon as it makes it harder to search code for uses of a pref. You can import Services.jsm and use `Services.prefs.getBoolPref("browser.formautofill.enabled")` instead.

::: browser/extensions/formautofill/FormAutofillParent.jsm:255
(Diff revision 2)
> +      formAutofillGroup,
> +      checkboxEnable,
> +      btnSavedProfiles

Nit: Can you rename these variables to be consistent with regarrds to where the element name is. Some have it at the beginning and others at the end.

::: browser/extensions/formautofill/FormAutofillParent.jsm:260
(Diff revision 2)
> +      formAutofillGroup,
> +      checkboxEnable,
> +      btnSavedProfiles
> +    };
> +
> +    formAutofillGroup.setAttribute("id", "formAutofillGroup");

.id =

::: browser/extensions/formautofill/FormAutofillParent.jsm:260
(Diff revision 2)
> +    formAutofillGroup.setAttribute("id", "formAutofillGroup");
> +    formAutofillGroup.setAttribute("data-category", "panePrivacy");
> +    formAutofillGroup.setAttribute("hidden", !this.isPrivacyPane);

Prefer using setters on the elements over `setAttribute` (except when dealing with potentially display:none XBL bindings) as it's supposedly more performant

::: browser/extensions/formautofill/FormAutofillParent.jsm:262
(Diff revision 2)
> +      btnSavedProfiles
> +    };
> +
> +    formAutofillGroup.setAttribute("id", "formAutofillGroup");
> +    formAutofillGroup.setAttribute("data-category", "panePrivacy");
> +    formAutofillGroup.setAttribute("hidden", !this.isPrivacyPane);

Please add a mochitest-browser-chrome test for both cases

::: browser/extensions/formautofill/FormAutofillParent.jsm:307
(Diff revision 2)
> +      case "command": {
> +        let target = aEvent.target;
> +
> +        if (target == this.refs.checkboxEnable) {
> +          // Set preference directly instead of relying on <Preference>
> +          this.prefService.setBoolPref("enabled", target.checked);

Don't use the pref branch here either

::: browser/extensions/formautofill/FormAutofillParent.jsm:309
(Diff revision 2)
> +
> +        if (target == this.refs.checkboxEnable) {
> +          // Set preference directly instead of relying on <Preference>
> +          this.prefService.setBoolPref("enabled", target.checked);
> +        } else if (target == this.refs.btnSavedProfiles) {
> +          // Open Saved Profiles dialog

add "TODO:" to the front to make it stand out

Comment 8

[Scott Wu [:scottwu]]

Comment on attachment 8822626 [details]
Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

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

Comment 9

[Scott Wu [:scottwu]]

Comment on attachment 8822626 [details]
Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

https://reviewboard.mozilla.org/r/101504/#review106198

> Please add a mochitest-browser-chrome test for both cases

Is there any way I can set prefs before starting a browser chrome test? Normally I would enable the "browser.formautofill.experimental" and restart the browser so that FormAutofillParent.init is called in bootstrap, but I am not sure how to do that here (in add_task?).

Comment 10

[Matthew N. [:MattN]]

Comment on attachment 8822626 [details]
Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

https://reviewboard.mozilla.org/r/101504/#review106198

> Is there any way I can set prefs before starting a browser chrome test? Normally I would enable the "browser.formautofill.experimental" and restart the browser so that FormAutofillParent.init is called in bootstrap, but I am not sure how to do that here (in add_task?).

See https://bugzilla.mozilla.org/show_bug.cgi?id=1304634#c34 and https://reviewboard.mozilla.org/r/100744/diff/1#index_header which I think will set it for all mochitests. You will need to switch to .experimental form .enabled.

Comment 11

[Matthew N. [:MattN]]

Comment on attachment 8822626 [details]
Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

https://reviewboard.mozilla.org/r/101504/#review106198

> See https://bugzilla.mozilla.org/show_bug.cgi?id=1304634#c34 and https://reviewboard.mozilla.org/r/100744/diff/1#index_header which I think will set it for all mochitests. You will need to switch to .experimental form .enabled.

We want to set that pref in prefs_general.js anyways since we need it for mochitest-plain but if that doesn't work for mochitest-browser-chrome then I'm also fine with directly calling FormAutoFillParent.init()

Comment 12

[Matthew N. [:MattN]]

Comment on attachment 8822626 [details]
Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

https://reviewboard.mozilla.org/r/101504/#review107750

Thanks Scott.

Add a mochitest-browser-chrome checking:
* The intiial state of the checkbox when the pref is true
* The intiial state of the checkbox when the pref is true
* Visibility of the group when about:preferences is opened directly to the privacy pane
* Visibility of the group when about:preferences is opened directly to a non-privacy pane

Please use add_task with generator functions and the helpers in BrowserTestUtils.jsm. You can identity other browser-chrome test in the tree with the `browser_*.js` name.

You can attach tests as a separate commit on this bug so that the r+ can remain on code part, if you would like.

::: browser/extensions/formautofill/FormAutofillParent.jsm:123
(Diff revision 3)
>  
>      let mm = Cc["@mozilla.org/globalmessagemanager;1"]
>                 .getService(Ci.nsIMessageListenerManager);
>      mm.removeMessageListener("FormAutofill:PopulateFieldValues", this);
> +
> +    Services.obs.removeObserver(this, "advanced-pane-loaded", false);

Remove the non-existant third argument.

::: browser/extensions/formautofill/FormAutofillParent.jsm:249
(Diff revision 3)
> +      savedProfilesBtn
> +    };
> +
> +    formAutofillGroup.id = "formAutofillGroup";
> +    formAutofillGroup.hidden = !this.isPrivacyPane;
> +    formAutofillGroup.setAttribute("data-category", "panePrivacy");

Nit: formAutofillGroup.dataset.category = "panePrivacy";

::: browser/extensions/formautofill/FormAutofillParent.jsm:252
(Diff revision 3)
> +    savedProfilesBtn.setAttribute("label", this.bundle.GetStringFromName("savedProfiles"));
> +    enabledCheckbox.setAttribute("label", this.bundle.GetStringFromName("enableProfileAutofill"));

Nit: .label for both of these too

::: browser/extensions/formautofill/locale/en-US/formautofill.properties:7
(Diff revision 3)
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +preferenceGroupTitle = Form Autofill
> +enableProfileAutofill = Enable Profile Autofill
> +savedProfiles = Saved Profiles...

Please use the single character ellipsis(…) instead of 3 periods.

Comment 13

[Scott Wu [:scottwu]]

Attachment: Bug 1304306 - (Part 2) Add mochitest-browser-chrome test for Form Autofill preferences

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

Comment 14

[Scott Wu [:scottwu]]

Comment on attachment 8822626 [details]
Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

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

Comment 15

[Scott Wu [:scottwu]]

Comment on attachment 8822626 [details]
Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

https://reviewboard.mozilla.org/r/101504/#review106198

> We want to set that pref in prefs_general.js anyways since we need it for mochitest-plain but if that doesn't work for mochitest-browser-chrome then I'm also fine with directly calling FormAutoFillParent.init()

prefs_general.js works! That's exactly what I need.

Comment 16

[Scott Wu [:scottwu]]

Comment on attachment 8822626 [details]
Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

https://reviewboard.mozilla.org/r/101504/#review107750

> Nit: formAutofillGroup.dataset.category = "panePrivacy";

I tried to do that but I couldn't use dataset for formAutofillGroup, so I don't think it's available to xul elements. I see that "data-category" was retrieved using getAttribute here so well and not with `.dataset`: http://searchfox.org/mozilla-central/rev/02a56df6474a97cf84d94bbcfaa126979970905d/browser/components/preferences/in-content/preferences.js#193

> Nit: .label for both of these too

I tried using .label but it didn't work. Then I realized it was caused by the problem you mentioned last time about setting attributes before the elements were constructed. In my current implementation, I create elements and set their attributes for the preference group, but only insert them into the document in the final step. If I were to make .label work, I would need to insert the elements first, before setting any attributes, but I wonder if it's worth changing the flow just to avoid using setAttribute?

> Please use the single character ellipsis(…) instead of 3 periods.

Thanks, will do that.

Comment 17

[Scott Wu [:scottwu]]

I've added browser-chrome tests in a separate commit. Please let me know if you see any problem. Thanks a lot Matt!

Comment 18

[Matthew N. [:MattN]]

Comment on attachment 8822626 [details]
Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

https://reviewboard.mozilla.org/r/101504/#review107750

> I tried to do that but I couldn't use dataset for formAutofillGroup, so I don't think it's available to xul elements. I see that "data-category" was retrieved using getAttribute here so well and not with `.dataset`: http://searchfox.org/mozilla-central/rev/02a56df6474a97cf84d94bbcfaa126979970905d/browser/components/preferences/in-content/preferences.js#193

Oh yeah, I forgot that it doesn't work in XUL. It's fine as-is then but maybe add a comment above it to say HTMLElement.dataset isn't available on XUL elements.

> I tried using .label but it didn't work. Then I realized it was caused by the problem you mentioned last time about setting attributes before the elements were constructed. In my current implementation, I create elements and set their attributes for the preference group, but only insert them into the document in the final step. If I were to make .label work, I would need to insert the elements first, before setting any attributes, but I wonder if it's worth changing the flow just to avoid using setAttribute?

Oh, I didn't realize that <button> was an XBL element. It's fine as-is then.

Comment 19

[Matthew N. [:MattN]]

Comment on attachment 8830598 [details]
Bug 1304306 - (Part 2) Add mochitest-browser-chrome test for Form Autofill preferences

https://reviewboard.mozilla.org/r/107340/#review111516

::: browser/extensions/formautofill/test/browser/browser_check_preferences.js:11
(Diff revision 1)
> +const CHECKBOX_AUTOFILL = GROUP_AUTOFILL + " checkbox";
> +const PREF_AUTOFILL_ENABLED = "browser.formautofill.enabled";
> +
> +// Visibility of form autofill group should be hidden when opening
> +// preferences page.
> +add_task(function*() {

Please name the task functions test_someDescriptionHere since it gets printed in the logs. With the name it oftens removes the need for a comment.

::: browser/extensions/formautofill/test/browser/browser_check_preferences.js:12
(Diff revision 1)
> +  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, PAGE_PREFS);
> +  is(gBrowser.contentDocument.querySelector(GROUP_AUTOFILL).hidden, true);
> +  yield BrowserTestUtils.removeTab(tab);

It would be preferred to use BrowserTestUtils.withNewTab here

::: browser/extensions/formautofill/test/browser/browser_check_preferences.js:13
(Diff revision 1)
> +
> +// Visibility of form autofill group should be hidden when opening
> +// preferences page.
> +add_task(function*() {
> +  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, PAGE_PREFS);
> +  is(gBrowser.contentDocument.querySelector(GROUP_AUTOFILL).hidden, true);

You should almost always use the last argument to describe what is being tested so the logs are more useful.

Comment 20

[Scott Wu [:scottwu]]

Comment on attachment 8822626 [details]
Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

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

Comment 21

[Scott Wu [:scottwu]]

Comment on attachment 8830598 [details]
Bug 1304306 - (Part 2) Add mochitest-browser-chrome test for Form Autofill preferences

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

Comment 23

[Matthew N. [:MattN]]

Comment on attachment 8830598 [details]
Bug 1304306 - (Part 2) Add mochitest-browser-chrome test for Form Autofill preferences

https://reviewboard.mozilla.org/r/107340/#review112220

::: browser/extensions/formautofill/test/browser/browser.ini:4
(Diff revision 2)
>  [DEFAULT]
>  
>  [browser_check_installed.js]
> +[browser_check_preferences.js]

Nit: browser_privacyPreferences.js would be clearer. The "check" here is redundant.

::: browser/extensions/formautofill/test/browser/browser_check_preferences.js:31
(Diff revision 2)
> +  });
> +});
> +
> +// Checkbox should be checked when form autofill is enabled.
> +add_task(function* test_autofillEnabledCheckbox() {
> +  Services.prefs.setBoolPref(PREF_AUTOFILL_ENABLED, true);

This is going to leave the preference in this state for other tests run in the same directory afterwards. If you want automatic cleanup you can use
```js
SpecialPowers.pushPrefEnv({set:[[PREF_AUTOFILL_ENABLED, true]]})
```

Comment 24

[Scott Wu [:scottwu]]

Comment on attachment 8822626 [details]
Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

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

Comment 25

[Scott Wu [:scottwu]]

Comment on attachment 8830598 [details]
Bug 1304306 - (Part 2) Add mochitest-browser-chrome test for Form Autofill preferences

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

Comment 26

[Scott Wu [:scottwu]]

Comment on attachment 8822626 [details]
Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/101504/diff/6-7/

Comment 27

[Scott Wu [:scottwu]]

Comment on attachment 8830598 [details]
Bug 1304306 - (Part 2) Add mochitest-browser-chrome test for Form Autofill preferences

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

Comment 28

[Scott Wu [:scottwu]]

Comment on attachment 8822626 [details]
Bug 1304306 - (Part 1) Add Form Autofill preference checkbox and button

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/101504/diff/7-8/

Comment 29

[Scott Wu [:scottwu]]

Comment on attachment 8830598 [details]
Bug 1304306 - (Part 2) Add mochitest-browser-chrome test for Form Autofill preferences

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

Comment 30

[Scott Wu [:scottwu]]

I've addressed the final issues and rebased to central. Checking it in.

Thanks :MattN for reviews and feedbacks!

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/89a029b06eb1
(Part 1) Add Form Autofill preference checkbox and button r=MattN
https://hg.mozilla.org/integration/autoland/rev/9b06a1b666e8
(Part 2) Add mochitest-browser-chrome test for Form Autofill preferences r=MattN

Comment 32

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for package failures and eslint failure:

https://hg.mozilla.org/integration/autoland/rev/da15441608ac1dccbb548fdc1fefb8c6a2a7f88a
https://hg.mozilla.org/integration/autoland/rev/10a2d778276f00ee1b85ca3861fb4259de811539

This already failed in the Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b021013e4586&selectedJob=76281691

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

Failure log bustage: https://treeherder.mozilla.org/logviewer.html#?job_id=76401327&repo=autoland

[task 2017-02-10T17:53:21.395878Z] 17:53:21     INFO -  gmake[7]: Entering directory '/home/worker/workspace/build/src/obj-firefox/browser/locales'
[task 2017-02-10T17:53:21.396121Z] 17:53:21     INFO -  /home/worker/workspace/build/src/obj-firefox/_virtualenv/bin/python /home/worker/workspace/build/src/toolkit/mozapps/installer/l10n-repack.py /home/worker/workspace/build/src/obj-firefox/dist/l10n-stage/firefox ../../dist/xpi-stage/locale-x-test \
[task 2017-02-10T17:53:21.396224Z] 17:53:21     INFO -  	 \
[task 2017-02-10T17:53:21.396373Z] 17:53:21     INFO -  Error: Locale doesn't have a manifest entry for 'formautofill'
[task 2017-02-10T17:53:21.396508Z] 17:53:21     INFO -  Traceback (most recent call last):
[task 2017-02-10T17:53:21.396657Z] 17:53:21     INFO -    File "/home/worker/workspace/build/src/toolkit/mozapps/installer/l10n-repack.py", line 60, in <module>
[task 2017-02-10T17:53:21.396768Z] 17:53:21     INFO -      main()
[task 2017-02-10T17:53:21.396991Z] 17:53:21     INFO -    File "/home/worker/workspace/build/src/toolkit/mozapps/installer/l10n-repack.py", line 56, in main
[task 2017-02-10T17:53:21.397178Z] 17:53:21     INFO -      non_resources=args.non_resource, non_chrome=NON_CHROME)
[task 2017-02-10T17:53:21.397372Z] 17:53:21     INFO -    File "/home/worker/workspace/build/src/python/mozbuild/mozpack/packager/l10n.py", line 257, in repack
[task 2017-02-10T17:53:21.397502Z] 17:53:21     INFO -      _repack(app_finder, l10n_finder, copier, formatter, non_chrome)
[task 2017-02-10T17:53:21.397622Z] 17:53:21     INFO -    File "/usr/lib/python2.7/contextlib.py", line 24, in __exit__
[task 2017-02-10T17:53:21.397741Z] 17:53:21     INFO -      self.gen.next()
[task 2017-02-10T17:53:21.397870Z] 17:53:21     INFO -    File "/home/worker/workspace/build/src/python/mozbuild/mozpack/errors.py", line 131, in accumulate
[task 2017-02-10T17:53:21.397993Z] 17:53:21     INFO -      raise AccumulatedErrors()
[task 2017-02-10T17:53:21.398117Z] 17:53:21     INFO -  mozpack.errors.AccumulatedErrors
[task 2017-02-10T17:53:21.398248Z] 17:53:21     INFO -  /home/worker/workspace/build/src/toolkit/locales/l10n.mk:117: recipe for target 'repackage-zip' failed
[task 2017-02-10T17:53:21.398403Z] 17:53:21     INFO -  gmake[7]: *** [repackage-zip] Error 1

Failure log eslint: https://treeherder.mozilla.org/logviewer.html#?job_id=76401322&repo=autoland
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/browser/extensions/formautofill/FormAutofillParent.jsm:85:3 | Expected method shorthand. (object-shorthand)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/browser/extensions/formautofill/test/browser/browser_privacyPreferences.js:18:45 | 'args' is already declared in the upper scope. (no-shadow)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/browser/extensions/formautofill/test/browser/browser_privacyPreferences.js:29:45 | 'args' is already declared in the upper scope. (no-shadow)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/browser/extensions/formautofill/test/browser/browser_privacyPreferences.js:43:45 | 'args' is already declared in the upper scope. (no-shadow)

Comment 33

[Matthew N. [:MattN]]

Try push with a fix: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e269d0d85d3d2cc19264fa19388f5fb934855210

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6eb7c1487c1f
(Part 1) Add Form Autofill preference checkbox and button. r=MattN
https://hg.mozilla.org/integration/mozilla-inbound/rev/633aff36848d
(Part 2) Add mochitest-browser-chrome test for Form Autofill preferences. r=MattN

Comment 35

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/6eb7c1487c1f
https://hg.mozilla.org/mozilla-central/rev/633aff36848d

Comment 36

[Julien Cristau [:jcristau]]

As this include string changes I assume it's not a candidate for 53.

Comment 37

[Francesco Lodolo [:flod]]

These strings just popped up in my Nightly's preferences, and they are not exposed for localization. Is that on purpose, given the "Autofill of addresses is only ready for testing in en-US…" warning?

Comment 38

[Matthew N. [:MattN]]

Yes, it's on purpose for two main reasons:
1) We're shipping as a system extension so the strings will be localized outside of m-c and we didn't get all the logistics in place yet
2) We're still figuring out exactly which locales the feature will ship in and didn't want localizers unnecessarily translating strings that won't be shipped anytime soon.

Comment 39

[Adrian Florinescu [:aflorinescu]]

Adding verification flag for Nightly 56/20170712100301 (Ubuntu 16.04x64, OSX 10.12 and Windows 10x64.)