1125316 - KidFox: Restricted profiles - Hide other non-essential Settings items - Customize > Import from Android

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Profile Handling, defect)

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: 1125316_kidfox_settings_import.patch

This patch adds the already existing restriction DISALLOW_IMPORT_SETTINGS to the list of restrictions for restricted profiles.

In addition to that:
* Cleanup: Enums are always static and their constructor is automatically private
* Previously I'd always check whether Restriction.DISALLOW_TOOLS_MENU is in the list of restrictions, now I actually check the restriction that was passed in..

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=48aa3c9a6f51

Comment 3

[Allison Naaktgeboren :ally]

Comment on attachment 8631530 [details] [diff] [review]
1125316_kidfox_settings_import.patch

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

Almost there.

::: mobile/android/base/RestrictedProfiles.java
@@ +63,5 @@
>      /* This is a list of things we can restrict you from doing. Some of these are reflected in Android UserManager constants.
>       * Others are specific to us.
>       * These constants should be in sync with the ones from toolkit/components/parentalcontrols/nsIParentalControlServices.idl
>       */
> +    public enum Restriction {

> * Cleanup: Enums are always static and their constructor is automatically private

I don't mean to be dense, but your comment says you made the enums static. As far as I can tell, you actually removed the static keyword, so how are these static now?

@@ +81,5 @@
>  
>          public final int id;
>          public final String name;
>  
> +        Restriction(final int id, final String name) {

> * Cleanup: Enums are always static and their constructor is automatically private

What forces this constructor to be private?

@@ +225,5 @@
>  
>              return !restrictionsOfGuestProfile.contains(restriction);
>          }
>  
> +        // Hardcoded restrictions. Make restrictions configurable and read from UserManager (Bug 1180653)

I so look forward to bug 1180653 :)

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

The main reason why I did the two changes to the enum was to get rid of the warnings/hints in IntelliJ:

(In reply to Allison Naaktgeboren please NEEDINFO? :ally from comment #3)
> I don't mean to be dense, but your comment says you made the enums static.
> As far as I can tell, you actually removed the static keyword, so how are
> these static now?

Sorry for the confusion I have caused. What I wanted to say is that the 'static' keyword is redundant because an inner-class enum is always/implicitly static: http://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#jls-8.9

(In reply to Allison Naaktgeboren please NEEDINFO? :ally from comment #3)
> What forces this constructor to be private?

The compiler will throw an error when an enum constructor is defined as public or protected, making the constructor implicitly private: http://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#jls-8.9.2 AFAIK it's not possible to define an enum constant at runtime invoking some enum constructor from outside code.

(In reply to Allison Naaktgeboren please NEEDINFO? :ally from comment #3)
> I so look forward to bug 1180653 :)

Oh yeah, me too! I've already started working on that and will hopefully have a patch for you to review soon. :)

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8631530 [details] [diff] [review]
1125316_kidfox_settings_import.patch

Re-requesting review. See comments above. :)

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

url:        https://hg.mozilla.org/integration/fx-team/rev/9e75c8987c3e47e22de600aadd3398429674ad87
changeset:  9e75c8987c3e47e22de600aadd3398429674ad87
user:       Sebastian Kaspari <[e-mail]>
date:       Sat Jul 18 12:41:24 2015 +0200
description:
Bug 1125316 - Restricted profiles: Hide setting "Customize > Import from Android". r=ally

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/9e75c8987c3e

Comment 8

[Catalin Suciu [:csuciu]]

Verified on Nightly and Aurora that disabling 'Import from Android' as owner, will hide this option on restricted profiles