KidFox: Disable first-run screen for restricted profile

VERIFIED FIXED in Firefox 42

Status

()

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: sfang, Assigned: ally)

Tracking

42 Branch
Firefox 42
Unspecified
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 verified)

Details

Attachments

(1 attachment)

Comment hidden (empty)
Assignee: nobody → randersen
Is there a specific reason why we want to have a splash screen in KidFox? I feel like especially in browsers where you switch to from various apps, a splash screen might slow you down noticeable.
(In reply to :Sebastian Kaspari from comment #1)
> Is there a specific reason why we want to have a splash screen in KidFox? I
> feel like especially in browsers where you switch to from various apps, a
> splash screen might slow you down noticeable.

Splash screens aren't typical on Android, but we could add some basic information for the First Run Experience (What Kinderfox is, how to use it, how it's helpful, etc). If this is strictly a branding screen we may need to talk to Martell and Co. 

The splash screen in any case should only show on the first load and not when returning to the app from switching.
(In reply to Robin Andersen [:tecgirl] from comment #2)
> (In reply to :Sebastian Kaspari from comment #1)
> > Is there a specific reason why we want to have a splash screen in KidFox? I
> > feel like especially in browsers where you switch to from various apps, a
> > splash screen might slow you down noticeable.
> 
> Splash screens aren't typical on Android, but we could add some basic
> information for the First Run Experience (What Kinderfox is, how to use it,
> how it's helpful, etc). If this is strictly a branding screen we may need to
> talk to Martell and Co. 

I interpreted "splash screen" to mean something similar to our first run page. We could also use the overlay dialog pattern that antlam and mhaigh worked on for the "try out tab queues" and "now there's tracking protection in pb" prompts.

> The splash screen in any case should only show on the first load and not
> when returning to the app from switching.

+1 to first load. Does this mean we would only ever display it once, similar to our first run experience?
Summary: KidFox: Splash screen for restricted profile → KidFox: First-run screen for restricted profile
We should have the first app run experience (import history/bookmarks from other browsers, sync feature, see https://bugzilla.mozilla.org/show_bug.cgi?id=1172623) not show up for the kids version, so either check on startup for restricted profile and swap the first app run experience, or just use the splash screen only for kidfox and no first run.
(Reporter)

Updated

4 years ago
Status: NEW → ASSIGNED
I think we should consider just dropping the firstrun for restricted profiles. If we have enough time, we could display a simple splashscreen, but I question the value in that as well.
Or to put another way, the real bug we need to fix here is to not show the firstrun panel in restricted profiles.
Assignee: randersen → nobody
Status: ASSIGNED → NEW
(In reply to Allison Naaktgeboren please NEEDINFO? :ally from comment #7)
> Created attachment 8638838 [details] [diff] [review]
> Kidfox-noadultfirstrun

I would ask liuche for advice about the best way to be doing this. Also, we should re-summarize this bug if it's going to morph to be about hiding the first run experience.
(Assignee)

Updated

4 years ago
Attachment #8638838 - Flags: review?(s.kaspari)
Comment on attachment 8638838 [details] [diff] [review]
Kidfox-noadultfirstrun

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

LGTM.

::: mobile/android/base/BrowserApp.java
@@ +2632,5 @@
>          }
>      }
>  
>      private void showFirstrunPager() {
> +        // Do not show first run if we're in an Android  Restricted Profile

NIT: There are two spaces after 'Android' :)

@@ +2633,5 @@
>      }
>  
>      private void showFirstrunPager() {
> +        // Do not show first run if we're in an Android  Restricted Profile
> +        if (RestrictedProfiles.isUserRestricted(this)) {

isUserRestricted() will return true for guest profiles too. But I guess here this won't be a problem as we don't show any first run screen for guests anyways.

@@ +2634,5 @@
>  
>      private void showFirstrunPager() {
> +        // Do not show first run if we're in an Android  Restricted Profile
> +        if (RestrictedProfiles.isUserRestricted(this)) {
> +          return;

NIT: The Java/Android code style is to indent with 4 spaces
Attachment #8638838 - Flags: review?(s.kaspari) → review+
(Assignee)

Comment 10

4 years ago
I understand at this morning's kidfox meeting there was a request for a single fistrun panel with some text on it?
Flags: needinfo?(jchaulk)
(In reply to Allison Naaktgeboren please NEEDINFO? :ally from comment #10)
> I understand at this morning's kidfox meeting there was a request for a
> single fistrun panel with some text on it?

We still want to disable the first run experience as a minimal fix.

In the meeting this morning, Sam and Robin talked about reaching out to Matej to come up with some content for a restricted profile first run experience, but we do not want to block on that for shipping in 42. This is just a stretch goal if we get the copy in time, so I would file a separate bug for this.
Assignee: nobody → ally
Summary: KidFox: First-run screen for restricted profile → KidFox: Disable first-run screen for restricted profile
Comment on attachment 8638838 [details] [diff] [review]
Kidfox-noadultfirstrun

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

Drive-by: We could do this even earlier in `checkFirstrun` to skip a pref check:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java#905
(Assignee)

Comment 13

4 years ago
(In reply to :Margaret Leibovic from comment #12)
> Comment on attachment 8638838 [details] [diff] [review]
> Kidfox-noadultfirstrun
> 
> Review of attachment 8638838 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Drive-by: We could do this even earlier in `checkFirstrun` to skip a pref
> check:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.
> java#905

I was concerned about the behavior in that one resulting in unexpected first runs elsewhere, as summed up by the comment:

910             // Note that we don't set the pref, so subsequent launches can result
911             // in the firstrun pane being shown.
(In reply to Allison Naaktgeboren please NEEDINFO? :ally from comment #13)
> (In reply to :Margaret Leibovic from comment #12)
> > Comment on attachment 8638838 [details] [diff] [review]
> > Kidfox-noadultfirstrun
> > 
> > Review of attachment 8638838 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Drive-by: We could do this even earlier in `checkFirstrun` to skip a pref
> > check:
> > http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.
> > java#905
> 
> I was concerned about the behavior in that one resulting in unexpected first
> runs elsewhere, as summed up by the comment:
> 
> 910             // Note that we don't set the pref, so subsequent launches
> can result
> 911             // in the firstrun pane being shown.

Well they wouldn't result in the first run being shown if we're going through the same restricted profile check every time, since we only ever decide to show the first run UI from this method.
https://hg.mozilla.org/mozilla-central/rev/f327907afbb1
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
(Reporter)

Updated

4 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Updated

4 years ago
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
(Reporter)

Comment 17

4 years ago
sorry, I changed status on the wrong bug, please ignore what I did.
(Assignee)

Updated

4 years ago
Flags: needinfo?(jchaulk)

Comment 18

4 years ago
Verified as fixed on latest Aurora
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.