1064177 - Synced Tabs panel shouldn't be present in Guest Mode

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Teodora Vermesan (:TeoVermesan)]

Tested with:
Build: Firefox for Android 35.0a1 (2014-09-07)
Device: Samsung Galaxy S5
OS: Android 4.4.2

Steps to reproduce:
1. Go to Tools -> New Guest Session

Expected results:
- In about:home there are only 5 default panels displayed. "Synced Tabs" is not present, since the Sync Tabs section is hidden in the tab tray.

Actual results:
- Synced Tabs panel appears along with the other 5 panels.

Comment 1

[Aaron Train [:aaronmt]]

Tracking for parity with the with the logic via old tab panel?

Comment 2

[Nick Alexander :nalexander [he/him]]

(In reply to Aaron Train [:aaronmt] from comment #1)
> Tracking for parity with the with the logic via old tab panel?

I agree.   I was only vaguely aware of this logic for guest mode.  Thanks for catching this, folks.

Comment 3

[Nick Alexander :nalexander [he/him]]

Attachment: Don't show Synced Tabs home panel in Guest Mode. r=wesj

We hard-code a number of default panel configuration options here, so
let's add another.

I don't care to migrate serialized Guest profile Home Panel
configurations forward.  I don't expect there are any: no Home Panel
configuration is written to SharedPreferences until the user edits the
default configuration in Settings > Customize > Home, and I expect ~0
folks to have taken these steps while in Guest Mode.

wesj: I see no reason to block this for the Restricted Profiles
landing, since this is very unlikely to cause significant changes to
that patch.

Comment 4

[Wesley Johnston (:wesj)]

Comment on attachment 8486569 [details] [diff] [review]
Don't show Synced Tabs home panel in Guest Mode. r=wesj

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

::: mobile/android/base/home/HomeConfigPrefsBackend.java
@@ +84,5 @@
>          final PanelConfig recentTabsEntry = createBuiltinPanelConfig(mContext, PanelType.RECENT_TABS);
> +
> +        // We disable Synced Tabs for guest mode profiles.
> +        final PanelConfig remoteTabsEntry;
> +        if (!GeckoProfile.get(mContext).inGuestMode()) {

We've moved to restricted profiles for this stuff in Java. i.e. this should be:

RestrictedProfiles.isAllowed(RestrictedProfiles.Restriction.SYNC);

Comment 5

[Nick Alexander :nalexander [he/him]]

Attachment: Part 1: Add RestrictedProfiles.DISALLOW_MODIFY_ACCOUNTS. r=wesj

This corresponds to UserManager.DISALLOW_MODIFY_ACCOUNTS.

Comment 6

[Nick Alexander :nalexander [he/him]]

Attachment: Part 2: Don't show Synced Tabs home panel in Guest Mode. r=wesj

We hard-code a number of default panel configuration options here, so
let's add another.

I don't care to migrate serialized Guest profile Home Panel
configurations forward.  I don't expect there are any: no Home Panel
configuration is written to SharedPreferences until the user edits the
default configuration in Settings > Customize > Home, and I expect ~0
folks to have taken these steps while in Guest Mode.

While I was here, I updated the other uses of GeckoProfile.inGuestMode
to guard Sync related things that I found.

wesj: the idiom we've defined with isAllowed(...DISALLOW_) reads
poorly.  I don't know what you and mfinkle were going for, but this is
awkward and will lead to bugs later.

Also, I removed a static reference to UserManager.DISALLOW_*.  I'm not
confident that does the right thing pre v18.  Can you explain why dex
accepts this?  Is the constant defined at compile time only, and
there's no runtime reference?

Comment 7

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8490391 [details] [diff] [review]
Part 2: Don't show Synced Tabs home panel in Guest Mode. r=wesj

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

Sorry, I meant to flag you for this too.

Comment 8

[Wesley Johnston (:wesj)]

Comment on attachment 8490391 [details] [diff] [review]
Part 2: Don't show Synced Tabs home panel in Guest Mode. r=wesj

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

::: mobile/android/base/home/HomeConfigPrefsBackend.java
@@ +13,5 @@
>  
>  import org.json.JSONArray;
>  import org.json.JSONException;
>  import org.json.JSONObject;
> +import org.mozilla.gecko.GeckoProfile;

Do you use this?

::: mobile/android/base/preferences/GeckoPreferences.java
@@ +713,5 @@
>                      CharSequence selectedEntry = listPref.getEntry();
>                      listPref.setSummary(selectedEntry);
>                      continue;
> +                } else if (PREFS_SYNC.equals(key) &&
> +                           !RestrictedProfiles.isAllowed(RestrictedProfiles.Restriction.DISALLOW_MODIFY_ACCOUNTS)) {

I'm a little worried about using this restriction. I wonder if a parent would someday sync a kids account between desktop and tablet for a kid, but would not want the kid to be able to set up the same sync. They shouldn't be able to modify the account, but they can have it. i.e. these restrictions should all be (!syncSetup && !allowModifyAccounts).

BUT, that's a bridge I'm happy to tackle when we cross it. Nothing here makes it any harder.

Comment 9

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/integration/fx-team/rev/3b86d4a1dd9f
https://hg.mozilla.org/integration/fx-team/rev/67f1009c3eba

Comment 10

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/3b86d4a1dd9f
https://hg.mozilla.org/mozilla-central/rev/67f1009c3eba