Closed Bug 786095 Opened 7 years ago Closed 7 years ago

Disable Social API when in safe mode

Categories

(Firefox Graveyard :: SocialAPI, defect)

defect
Not set

Tracking

(firefox17 verified)

RESOLVED FIXED
Firefox 18
Tracking Status
firefox17 --- verified

People

(Reporter: Felipe, Assigned: jaws)

Details

(Whiteboard: [Fx17])

Attachments

(1 file, 3 obsolete files)

When sessionrestore takes over to prevent a sequence of crashes, social features should also be prevented to load, because it could be the content of the panels, the worker or the sidebar that causes the crash, leaving the user with an unrecoverable browser.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Whiteboard: [Fx17]
We should probably also disable social functionality in safe mode.
Summary: When sessionrestore takes over to prevent a crash, social features should also be disabled → Disable Social API when in session restore or safe mode
Attached patch Patch (obsolete) — Splinter Review
This patch works with disabling the Social API when in safe mode.

I need some more time to investigate restoring it upon Session Restore, since Session Restore is optional and the user can continue with a new session without performing any explicit action.

I think we can land this now though and move the Session Restore handling to a separate bug.
Attachment #656642 - Flags: review?(felipc)
If I'm not mistaken, that approach will have a bit of a weird effect - social UI won't show up in safe mode, but assuming social has been previously activated the "Motown for Nightly" menu item might still be checked, and toggling it will cause the UI to re-appear, right?

I think we need to "disable" it at a lower level (like socialservice.enabled, maybe?)
Attachment #656642 - Flags: review?(felipc)
This patch ended up being more complex than I thought it would need to be. I will also attach a simpler patch that doesn't allow for re-enabling. 

There are a few places that check the enabled pref and in order to allow the feature to be re-enabled while in safe mode I had to introduce an extra state variable to Social.jsm.

I also removed the check for setting the pref to the same value. When safe mode is entered, the feature is disabled even if the pref is set to true. Re-enabling the pref would keep it as true, so we can't early-return in this case.
Attachment #656642 - Attachment is obsolete: true
Attachment #658095 - Flags: review?(felipc)
This patch is simpler and is my preferred patch when compared to the previous patch.

When in safe mode, the Social features are disabled and do not have a visible way to enable them. Users will have to exit safe mode to enable the features.

The Social features could still be enabled/disabled via about:config, but enabling Social this way may produce unexpected behavior while in safe mode.
Attachment #658097 - Flags: review?(felipc)
Would just making SocialService.enabled always return false in safe mode work?
The menu items check for Social.active, so only dealing with SocialService.enabled isn't enough.
The menu item is kind of a special case - it's used for toggling, which we want to disable in safe mode. So we can just have it check Social.active && !safeMode, right?
It'd be nice to have a way in safemode to disable the feature for the next non-safemode run. Is that gonna be possible with this patch? From what I can tell it will
I talked with Gavin, Felipe, and MattN about this patch over the past 2 weeks.

Getting this to work correctly is a little complicated, but this patch is also quite small.

The way that this works is:
- If the feature is enabled prior to safe mode
--- The menuitem will show it as being enabled
--- All other Social API features will be disabled
--- The user can disable Social API by unchecking the menuitem
--- The user can re-enable Social API (and its full UI) by re-checking the menuitem

- If the feature is disabled prior to safe mode
--- The menuitem will be unchecked when safe mode is started
--- The full Social API can be enabled by checking the menuitem

This will allow a user who is experiencing issues to restart in safe mode, and on-demand disable & enable the Social API as they are trying to narrow down the cause of their issues.
Attachment #658095 - Attachment is obsolete: true
Attachment #658097 - Attachment is obsolete: true
Attachment #658095 - Flags: review?(felipc)
Attachment #658097 - Flags: review?(felipc)
Attachment #663594 - Flags: review?(felipc)
Attachment #663594 - Flags: review?(felipc) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/32d1752fb7ba
Flags: in-testsuite-
Summary: Disable Social API when in session restore or safe mode → Disable Social API when in safe mode
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/d12c34bb82fa for causing:

TEST-UNEXPECTED-FAIL | /builds/slave/talos-slave/test/build/xpcshell/tests/toolkit/components/social/test/xpcshell/test_SocialService.js | test failed (with xpcshell return code: 0), see following log:
TEST-UNEXPECTED-FAIL | /builds/slave/talos-slave/test/build/xpcshell/tests/toolkit/components/social/test/xpcshell/test_SocialService.js | null != null - See following stack:
Landed again with a fix for the test (ok'd in person by Felipe).

https://hg.mozilla.org/integration/mozilla-inbound/rev/6c68ef18d340
https://hg.mozilla.org/mozilla-central/rev/6c68ef18d340
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
Attachment #663594 - Flags: approval-mozilla-aurora+
Verified on Firefox 17 beta 5 that Social API is disabled in safe mode and works as mentioned in Comment 10.

Verified on Windows 7, Ubuntu 12.04 and Mac OS X 10.8:
Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/17.0 Firefox/17.0
Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/17.0 Firefox/17.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/17.0 Firefox/17.0
mass remove verifyme requests greater than 4 months old
Keywords: verifyme
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.