1180795 - KidFox: Disable first-run screen for restricted profile

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Profile Handling, defect)

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

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.

Comment 2

[Robin Andersen [:tecgirl]]

(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.

Comment 3

[:Margaret Leibovic]

(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?

Comment 4

[Barbara Bermes [:barbara] - NI please!]

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.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

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.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Or to put another way, the real bug we need to fix here is to not show the firstrun panel in restricted profiles.

Comment 7

[Allison Naaktgeboren :ally]

Attachment: Kidfox-noadultfirstrun

Comment 8

[:Margaret Leibovic]

(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.

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

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

Comment 10

[Allison Naaktgeboren :ally]

I understand at this morning's kidfox meeting there was a request for a single fistrun panel with some text on it?

Comment 11

[:Margaret Leibovic]

(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.

Comment 12

[:Margaret Leibovic]

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

Comment 13

[Allison Naaktgeboren :ally]

(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.

Comment 14

[Pulsebot]

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

Comment 15

[:Margaret Leibovic]

(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.

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f327907afbb1

Comment 17

[Sam Fang [:sfang]]

sorry, I changed status on the wrong bug, please ignore what I did.

Comment 18

[u549602]

Verified as fixed on latest Aurora