784602 - dom.identity.enabled pref missing from about:config

Status: RESOLVED FIXED

(Core Graveyard :: Identity, defect)

Description

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

The patch for bug 753239 references a pref, dom.identity.enabled, that is missing from /modules/libpref/src/init/all.js.

The patch also added 'toolkit.identity.debug', which should be added as well.

These prefs should be added to all.js so they will be visible in about:config.

Comment 1

[Zuhao(Joe) Chen]

Thank you for the email, Jared. Can I start working on this?

Comment 2

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

Go for it! :)

Comment 3

[Zuhao(Joe) Chen]

Attachment: patch for bug 784602

I put those two prefs at the end of the file. I'm not really sure it's the right place though. Should those two prefs be set to true by default?

Comment 4

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

Comment on attachment 656657 [details] [diff] [review]
patch for bug 784602

The bottom of the file seems fine to me. I think both of these prefs should be false by default. Please remove the added comment, and make sure to keep a newline at the end of the file.

When you upload your next version of the patch, please request review from [e-mail]

Comment 5

[Zuhao(Joe) Chen]

Attachment: patch for bug 784602

Comment 6

[Matthew N. [:MattN]]

Comment on attachment 656664 [details] [diff] [review]
patch for bug 784602

Review of attachment 656664 [details] [diff] [review]:
-----------------------------------------------------------------

::: modules/libpref/src/init/all.js
@@ +3689,5 @@
>  // Disable idle observer fuzz, because only privileged content can access idle
>  // observers (bug 780507).
>  pref("dom.idle-observers-api.fuzz_time.disabled", true);
> +
> +pref("dom.identity.enabled", false);

Since identity UI only exists for the desktop browser, I don't think there's much value in exposing this pref for all Gecko-based apps because flipping it would be a footgun that would break all Persona sites.  We can move the dom.identity.enabled pref to browser/app/profile/firefox.js so it's only for Firefox. Leaving the debug pref here is fine.

Comment 7

[Zuhao(Joe) Chen]

Attachment: patch for bug 784602

No wonder there are not showing in the about:config in firefox. So what's the chrome url for modifying the prefs in all.js?

Comment 8

[Matthew N. [:MattN]]

Comment on attachment 656675 [details] [diff] [review]
patch for bug 784602

Review of attachment 656675 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Zuhao(Joe) Chen from comment #7)
> No wonder there are not showing in the about:config in firefox. So what's
> the chrome url for modifying the prefs in all.js?

You need to rebuild libpref for the changes:  make -C modules/libpref/ from your obj. dir.

Comment 9

[Zuhao(Joe) Chen]

(In reply to Matthew N. [:MattN] from comment #8)
> Comment on attachment 656675 [details] [diff] [review]
> patch for bug 784602
> 
> Review of attachment 656675 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> (In reply to Zuhao(Joe) Chen from comment #7)
> > No wonder there are not showing in the about:config in firefox. So what's
> > the chrome url for modifying the prefs in all.js?
> 
> You need to rebuild libpref for the changes:  make -C modules/libpref/ from
> your obj. dir.

I see. Thank you. :)

Comment 10

[Matthew N. [:MattN]]

Thanks for the patch.

Can you set the commit message on the patch to follow the guidelines here: https://developer.mozilla.org/en-US/docs/Developer_Guide/Committing_Rules_and_Responsibilities#Commit_message_restrictions

ie. 

Bug 784602 - Add identity prefs to about:config. r=MattN

Then you can set the flag "checkin?" on that new version.

Comment 11

[Zuhao(Joe) Chen]

Attachment: patch for bug 784602

Comment 12

[Matthew N. [:MattN]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ed82d3fb789c

Try results: https://tbpl.mozilla.org/?tree=Try&rev=fbe522587fcc

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ed82d3fb789c