Kidfox theme should not be present in guest mode session

VERIFIED FIXED in Firefox 42

Status

()

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: TeoVermesan, Assigned: sebastian)

Tracking

43 Branch
Firefox 43
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 verified, firefox43 verified)

Details

Attachments

(3 attachments)

(Reporter)

Description

4 years ago
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
Blocks: FFB, kidfox-v1
Oh that's interesting. I tested exactly this.
Assignee: nobody → s.kaspari
We just uplifted this so 42 is probably affected too.
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.
Blocks: 1182514
Status: NEW → ASSIGNED
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).
Attachment #8654915 - Flags: review?(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.
Attachment #8654915 - Flags: review?(ally) → review+
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
Attachment #8654915 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/9fff9644e1a0
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
(Reporter)

Comment 9

4 years ago
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)
Sebastian, is nsIParentalControlsService.idl a new file in 42? Thanks
Flags: needinfo?(s.kaspari)
(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.
Flags: needinfo?(s.kaspari)
OK! Thanks.
Jorge, is this change ok with you? Thanks
Flags: needinfo?(jorge)
Looks good to me.
Flags: needinfo?(jorge)
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
Attachment #8654915 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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!
Flags: needinfo?(s.kaspari)
This is the modified patch to apply cleanly to aurora.
Flags: needinfo?(s.kaspari) → needinfo?(cbook)
Patch can be applied but I am not seeing the theme when building from aurora branch.
Flags: needinfo?(cbook) → needinfo?(s.kaspari)
(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 :)
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).
Flags: needinfo?(teodora.vermesan)
Flags: needinfo?(s.kaspari)
cc'ing Erin in case #comment 19 is a 42 regression
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.
Flags: needinfo?(cbook)
(Reporter)

Comment 23

4 years ago
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
Flags: needinfo?(teodora.vermesan)
Flags: needinfo?(cbook)
(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

4 years ago
depends on #1205274

Updated

4 years ago
Depends on: 1205274

Comment 26

4 years ago
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)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.