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

VERIFIED FIXED in Firefox 42

Status

()

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: jchaulk, Assigned: sebastian)

Tracking

Trunk
Firefox 42
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 verified)

Details

Attachments

(1 attachment)

Comment hidden (empty)
Blocks: FFB
OS: Windows 8.1 → Android
Hardware: x86_64 → All
Version: unspecified → Trunk
(Reporter)

Updated

4 years ago
Blocks: kidfox-v1
Assignee: nobody → s.kaspari
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..
Attachment #8631530 - Flags: review?(ally)
Status: NEW → ASSIGNED
Blocks: 1125312
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 :)
Attachment #8631530 - Flags: review?(ally) → feedback+
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. :)
Flags: needinfo?(ally)
Comment on attachment 8631530 [details] [diff] [review]
1125316_kidfox_settings_import.patch

Re-requesting review. See comments above. :)
Flags: needinfo?(ally)
Attachment #8631530 - Flags: review?(ally)
Attachment #8631530 - Flags: review?(ally) → review+
url:        https://hg.mozilla.org/integration/fx-team/rev/9e75c8987c3e47e22de600aadd3398429674ad87
changeset:  9e75c8987c3e47e22de600aadd3398429674ad87
user:       Sebastian Kaspari <s.kaspari@gmail.com>
date:       Sat Jul 18 12:41:24 2015 +0200
description:
Bug 1125316 - Restricted profiles: Hide setting "Customize > Import from Android". r=ally
https://hg.mozilla.org/mozilla-central/rev/9e75c8987c3e
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Verified on Nightly and Aurora that disabling 'Import from Android' as owner, will hide this option on restricted profiles
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.