Closed Bug 1064177 Opened 5 years ago Closed 5 years ago

Synced Tabs panel shouldn't be present in Guest Mode

Categories

(Firefox for Android :: General, defect)

35 Branch
ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 35
Tracking Status
firefox35 --- verified
fennec 35+ ---

People

(Reporter: TeoVermesan, Assigned: nalexander)

References

Details

Attachments

(2 files, 1 obsolete file)

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.
Blocks: remotetabs
Tracking for parity with the with the logic via old tab panel?
tracking-fennec: --- → ?
(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.
Depends on: 1058150
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.
Attachment #8486569 - Flags: review?(wjohnston)
Assignee: nobody → nalexander
tracking-fennec: ? → 35+
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);
Attachment #8486569 - Flags: review?(wjohnston) → review-
This corresponds to UserManager.DISALLOW_MODIFY_ACCOUNTS.
Attachment #8490388 - Flags: review?(wjohnston)
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?
Attachment #8486569 - Attachment is obsolete: true
Attachment #8490391 - Flags: review+
Attachment #8490388 - Flags: review?(wjohnston) → review+
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.
Attachment #8490391 - Flags: review+ → review?(wjohnston)
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.
Attachment #8490391 - Flags: review?(wjohnston) → review+
https://hg.mozilla.org/mozilla-central/rev/3b86d4a1dd9f
https://hg.mozilla.org/mozilla-central/rev/67f1009c3eba
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.