Closed
Bug 1222381
Opened 9 years ago
Closed 9 years ago
Remove "Parental Controls Theme" for restricted profiles
Categories
(Firefox for Android Graveyard :: Family Friendly Browsing, defect)
Tracking
(firefox45 verified)
RESOLVED
FIXED
Firefox 45
Tracking | Status | |
---|---|---|
firefox45 | --- | verified |
People
(Reporter: sebastian, Assigned: sebastian)
References
Details
Attachments
(1 file)
40 bytes,
text/x-review-board-request
|
Details |
Currently we automatically install a greenish "Parental Controls Theme" (Lightweight theme) for restricted profiles. In bug 1220251 we want to introduce a theme picker. Let's remove our code for this override. There's no reason to switch from the "default" theme to a greenish theme and then let the user pick a theme.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → s.kaspari
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•9 years ago
|
||
Bug 1222381 - Remove default theme for restricted profiles. r?ally Instead we will introduce a theme picker in bug 1220251.
Attachment #8686004 -
Flags: review?(ally)
Updated•9 years ago
|
Attachment #8686004 -
Flags: review?(ally)
Comment 2•9 years ago
|
||
Comment on attachment 8686004 [details] MozReview Request: Bug 1222381 - Remove default theme for restricted profiles. r?ally https://reviewboard.mozilla.org/r/24939/#review22529 The patch is good, but before we actually ship it, we should settle the issue of programmatically determining guest browsing from FFB. r+ once we settle that. ::: mobile/android/chrome/content/browser.js (Diff revision 1) > - // We are using the DEFAULT_THEME restriction to differentiate between restricted profiles & guest mode - Bug 1199596 So.. this comment begs the question, if we get rid of the DISALLOW_DEFAULT_THEME restriction....how do we know the difference between guest & FBB anymore? ::: mobile/android/chrome/content/browser.js (Diff revision 1) > - _installParentalControlsTheme: function() { I'm using this, but I can pull it into my patch once you've landed this. Or we could just leave it. Up to you.
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Allison Naaktgeboren :ally from comment #2) > The patch is good, but before we actually ship it, we should settle the > issue of programmatically determining guest browsing from FFB. r+ once we > settle that. The thing is we currently do not need it on JS side. On Java side we have RestrictedProfile.isRestrictedProfile(). I can add it (as part of an other bug) if we need it - so far we only check some restrictions but not the state of the profile. > So.. this comment begs the question, if we get rid of the > DISALLOW_DEFAULT_THEME restriction....how do we know the difference between > guest & FBB anymore? This has only been some very ugly crutch for getting the default theme done. As mentioned above: Currently we do not need to distinguish (on JS side) because we just query whether a feature is allowed or not. > I'm using this, but I can pull it into my patch once you've landed this. Or > we could just leave it. Up to you. I assume for the panel you'll want to send an event to the JS side and call themeChanged() with a new theme configuration. There's probably no need to install it as a built-in (not removable) theme? I'd like to land this here and if we need to distinguish between guests and restricted profile (Do you have a use-case / bug that needs it currently?) then let's file a new bug and add this to nsIParentalControlsService.idl - the right way this time. :)
Flags: needinfo?(ally)
Comment 4•9 years ago
|
||
(In reply to Sebastian Kaspari (:sebastian) from comment #3) > (In reply to Allison Naaktgeboren :ally from comment #2) > > The patch is good, but before we actually ship it, we should settle the > > issue of programmatically determining guest browsing from FFB. r+ once we > > settle that. > > The thing is we currently do not need it on JS side. On Java side we have > RestrictedProfile.isRestrictedProfile(). I can add it (as part of an other > bug) if we need it - so far we only check some restrictions but not the > state of the profile. I fear this will come back to bit us, and me in particular on bug 1220251. Depends on where the mocks end up exactly. > > > I'm using this, but I can pull it into my patch once you've landed this. Or > > we could just leave it. Up to you. > > I assume for the panel you'll want to send an event to the JS side and call > themeChanged() with a new theme configuration. There's probably no need to > install it as a built-in (not removable) theme? If they're bundled with fennec, they're not removable anyway. The theme manager lets me switch between built in themes easily. > > I'd like to land this here and if we need to distinguish between guests and > restricted profile (Do you have a use-case / bug that needs it currently?) > then let's file a new bug and add this to nsIParentalControlsService.idl - > the right way this time. :) That's a plan I can roll with for the js side. You can haz r+
Flags: needinfo?(ally)
Assignee | ||
Comment 5•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/40e3ec35113722d706a62898082dfb0adeb9867c Bug 1222381 - Remove default theme for restricted profiles. r=ally
Comment 6•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/40e3ec351137
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Verified as fixed on latest Nightly (45.0a1 2015-11-22). Tested on Sony Xperia Z2 - Android 5.0.2
Comment 8•8 years ago
|
||
Barbara, if we're not going forward with the theme picker during first run (bug 1220251), should we back out this change?
Flags: needinfo?(bbermes)
Assignee | ||
Comment 9•8 years ago
|
||
(In reply to :Margaret Leibovic from comment #8) > Barbara, if we're not going forward with the theme picker during first run > (bug 1220251), should we back out this change? All the changes around restrictions (Reorganizing, Renaming, ..) will make this super painful to back out.
Comment 10•8 years ago
|
||
(In reply to Sebastian Kaspari (:sebastian) from comment #9) > (In reply to :Margaret Leibovic from comment #8) > > Barbara, if we're not going forward with the theme picker during first run > > (bug 1220251), should we back out this change? > > All the changes around restrictions (Reorganizing, Renaming, ..) will make > this super painful to back out. I actually feel like we should just forget about this and leave it alone, but I want it to be a conscious decision, not one we accidentally stumble into.
Comment 11•8 years ago
|
||
I'll provide more feedback once I talked to Karen re theme picker, however is it correct to assume if we don't do anything with this bug, the parental theme will remain the same and in the code?
Flags: needinfo?(s.kaspari)
Flags: needinfo?(margaret.leibovic)
Flags: needinfo?(bbermes)
Comment 12•8 years ago
|
||
(In reply to Barbara Bermes [:barbara] from comment #11) > I'll provide more feedback once I talked to Karen re theme picker, however > is it correct to assume if we don't do anything with this bug, the parental > theme will remain the same and in the code? We removed the built-in theme in this bug (anticipating we would have a theme chooser), so if we don't take more action, there won't be a custom theme applied to the restricted profiles.
Flags: needinfo?(margaret.leibovic)
Updated•8 years ago
|
Flags: needinfo?(s.kaspari)
Updated•6 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•