1199596 - Kidfox theme should not be present in guest mode session

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Teodora Vermesan (:TeoVermesan)]

Attachment: Screenshot_2015-08-28-09-11-35.png

Tested with:
Device: Alcatel One Touch (Android 4.1.2)
Build: Firefox for Android 43.0a1 (2015-08-27)

Steps to reproduce:
1. Open Nightly and go to Menu -> Tools -> New Guest Session

Actual results:
- Kidfox theme is installed for guest mode session

Expected results:
- Guest mode session should be displayed as normal browsing

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

Oh that's interesting. I tested exactly this.

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

We just uplifted this so 42 is probably affected too.

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

Meh. I can reproduce this and this is happening because ParentalControls.parentalControlsEnabled returns true for restricted and guest profiles. I'll need to do some research to see how I can work around this.

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: MozReview Request: Bug 1199596 - Only install "Parental Controls Theme" for restricted profiles and not guest profiles. r?ally

Bug 1199596 - Only install "Parental Controls Theme" for restricted profiles and not guest profiles. r?ally

From browser.js's point of view there's no difference between restricted and guest profiles. Both use the
parental controls API. So there are only two "simple" solutions here:

* 1) Add a method to nsIParentalControlsService to determine whether the current profiles is a restricted or
  a guest profile (Something like isGuest()). But then every platform using this interface would require
  to at least implement a stub for this method.

* 2) Add a new restriction that controls installing the theme.

This patch implements option 2. While this restriction is not of much use besides deciding whether we need
to install a specialized theme (DISALLOW_DEFAULT_THEME), it still offers the most flexibility. In a
follow-up bug we could decide to make the restriction configurable by the device admin (requires localized
strings).

Comment 5

[Allison Naaktgeboren :ally]

Comment on attachment 8654915 [details]
MozReview Request: Bug 1199596 - Only install "Parental Controls Theme" for restricted profiles and not guest profiles. r?ally

https://reviewboard.mozilla.org/r/17771/#review16041

I really don't care for this approach long term. I think it will bite us in the butt eventually. Now that we have two different restricted 'modes' it's unlikely we won't run into this situtation again.

However, since we'll probably want to uplift this, this is the safer option. The uuid change then is a concern, but I don't see a way around that this isn't really_awful.

Land this and please file a bug for option 1 while we're thinking about it so that this information can be properly exposed to gecko.

::: mobile/android/chrome/content/browser.js:3080
(Diff revision 1)
> -    if (ParentalControls.parentalControlsEnabled && !this._manager.currentTheme) {
> +    if (ParentalControls.parentalControlsEnabled &&

I think a comment is justified here to explain why we're using default_theme restriction to differentiate between kidfox & guest mode in JS.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/9fff9644e1a0

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8654915 [details]
MozReview Request: Bug 1199596 - Only install "Parental Controls Theme" for restricted profiles and not guest profiles. r?ally

Approval Request Comment
[Feature/regressing bug #]:Bug 1182514 - Light-weight kidfox theme (KidFox is targeting Fx42)
[User impact if declined]: Parental controls / KidFox theme will be installed when entering guest mode.
[Describe test coverage new/current, TreeHerder]: Manual testing. Not covered by UI tests.
[Risks and why]: Low risk. Checking restriction before installing theme.
[String/UUID change made/needed]: Patch updates UUID of toolkit/components/parentalcontrols/nsIParentalControlsService.idl

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Teodora Vermesan (:TeoVermesan)]

Kidfox theme is not present anymore in guest mode, so verified as fixed using:
Device: Nexus 4 (Android 5.1)
Build: Firefox for Android 43.0a1 (2015-09-03)

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Sebastian, is nsIParentalControlsService.idl a new file in 42? Thanks

Comment 11

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #10)
> Sebastian, is nsIParentalControlsService.idl a new file in 42? Thanks

No, this interface existed before and was at least used for guest mode on Android.

Comment 12

[Sylvestre Ledru [:Sylvestre]]

OK! Thanks.
Jorge, is this change ok with you? Thanks

Comment 13

[Jorge Villalobos [:jorgev] (he/him)]

Looks good to me.

Comment 14

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8654915 [details]
MozReview Request: Bug 1199596 - Only install "Parental Controls Theme" for restricted profiles and not guest profiles. r?ally

ok, let's take it then. thanks

Comment 15

[Carsten Book [:Tomcat]]

hi, this cause a conflict when applying to aurora:

merging mobile/android/base/restrictions/RestrictedProfileConfiguration.java
merging mobile/android/base/restrictions/RestrictionProvider.java
3 files to edit
merging mobile/android/base/restrictions/RestrictionProvider.java failed!
merging mobile/android/chrome/content/browser.js 

can you take a look, thanks!

Comment 16

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: 1199596-AURORA.patch

This is the modified patch to apply cleanly to aurora.

Comment 17

[Sebastian Kaspari (:sebastian; :pocmo)]

Patch can be applied but I am not seeing the theme when building from aurora branch.

Comment 18

[Carsten Book [:Tomcat]]

(In reply to Sebastian Kaspari (:sebastian) from comment #17)
> Patch can be applied but I am not seeing the theme when building from aurora
> branch.

ok holding off uplifting till you give green light for this :)

Comment 19

[Sebastian Kaspari (:sebastian; :pocmo)]

Can you reproduce this in 42? It seems like even we uplifted the theme to Aurora (bug 1182514) a lightweight theme is never applied (and also never for restricted profiles).

Comment 20

[Carsten Book [:Tomcat]]

cc'ing Erin in case #comment 19 is a 42 regression

Comment 21

[Sebastian Kaspari (:sebastian; :pocmo)]

Ok, I filed bug 1205274 for investigating the issue of applying the theme. It's not really related to the changes in this bug. It should be safe to uplift this patch here and continue in bug bug 1205274.

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/093bceb37663

Comment 23

[Teodora Vermesan (:TeoVermesan)]

Tested with a Nexus 7 (Android 5.1) with Aurora 42.0a1 (2015-09-15) a lightweight theme is never applied for restricted profiles: http://i.imgur.com/ig5JIgg.png

Comment 24

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Teodora Vermesan (:TeoVermesan) from comment #23)
> Tested with a Nexus 7 (Android 5.1) with Aurora 42.0a1 (2015-09-15) a
> lightweight theme is never applied for restricted profiles:
> http://i.imgur.com/ig5JIgg.png

Thank you!

Comment 25

[u549602]

depends on #1205274

Comment 26

[u549602]

Verified as fixed on both latest Nightly(44.0a1) and Aurora (43.0a2) on Alcatel One Touch (Android 4.1.2), Xiaomi Mi i4 (Android 5.0.2) and Sony Xperia Z2 (Android 5.0.2)