804242 - Tools > Social menu not showing up on FF 17/18 when prefs are manually flipped out of order

Status: RESOLVED WONTFIX

(Firefox Graveyard :: SocialAPI, defect)

Description

[Joe Drew (not getting mail)]

I have no Social menu under my Tools menu on OS X, but by all accounts I should have one. Same goes for sdwilsh and bwinton.

Comment 1

[Shane Caraveo (:mixedpuppy)]

This appears to be happening with recent nightly, and does not happen on beta or aurora.

Comment 3

[u279076]

FWIW, I don't have a Tools menu item on Nightly, Aurora, or Beta on Linux.

Comment 4

[Shane Caraveo (:mixedpuppy)]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #3)
> FWIW, I don't have a Tools menu item on Nightly, Aurora, or Beta on Linux.

in about:config, is social.activate true?  Did you activate all those through the facebook landing page?

Comment 5

[u279076]

I don't have a social.activate pref; I have social.active=true, if that's the same thing...

I activated Social through facebook.com/about/messenger-for-firefox.

Comment 6

[Shane Caraveo (:mixedpuppy)]

I've a feeling this is related to bug 772808, as well there is code working around this issue.  I'll dig into this more after meetings.

Comment 7

[Shane Caraveo (:mixedpuppy)]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #5)
> I don't have a social.activate pref; I have social.active=true, if that's
> the same thing...
> 
> I activated Social through facebook.com/about/messenger-for-firefox.

Yes, social.active.  Is that true for all varients (beta/aurora/nightly)?  I've verified my thought in comment 5, but that *only* affects nightly, and makes sense since nightly from Oct-18 showed the menu, whereas nightly from oct-20 does not.

Comment 8

[u279076]

social.active is set to true across all Firefox versions. I'm using 17.0b2, 18.0a2 2012-10-22, and 19.0a1 2012-10-22.

Comment 9

[Mark Hammond [:markh] [:mhammond]]

I can reproduce this manually.  Start FF with social.active and social.enabled set to false.  Set social.enabled to true, then set social.active to true - the sidebar etc appears, but the menu item does not show.  If you set the prefs in the reverse order, it works as expected.

I can't reproduce this using the activation feature, and looking at the code implies the order of the prefs should be set in the order that works - but it is possible there is a timing issue that could cause the pref changes to be *seen* in reverse which would account for it.

If people are seeing the menu not appear on aurora or beta even after a restart once everything is enabled, then it is likely to be a different problem.

Comment 10

[Mark Hammond [:markh] [:mhammond]]

Splitting this bug into 2.  This bug is for FF 17 and 18.  Bug 804442 is tracking the different problem happening currently on nightly.

Comment 11

[Shane Caraveo (:mixedpuppy)]

Attachment: prefs.patch

Other than the tests that are crashing due to bug 804558, existing tests all pass. Most of our tests add/remove providers and toggle these prefs, so I feel pretty confident that they are covering this change.

This also slightly relates to bug 804736, simply as an added assurance that our dual prefs are correctly toggled.

Comment 12

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 674462 [details] [diff] [review]
prefs.patch

+      // active always switches enabled, even to false, whereas enabled will
+      // ensure active is true if enabled is true

That seems a little confusing - isn't a simpler version just "Setting social.active must always cause Social.enabled to have the same value"?

Comment 13

[:Gavin Sharp [email: [e-mail]]]

The pref observer seems like overkill to me. Just the change to the "enabled" getter should be sufficient to avoid this problem, shouldn't it?

(Long-term, we probably want to get rid of social.active - it really was a hack limited to the single-provider activation scenario.)

Comment 14

[Mark Hammond [:markh] [:mhammond]]

FWIW, the observer means that when manually tweaking via about:config you see the effect of this "policy" (ie, manually setting .active to false also forces .enabled to false as you change it).  IIUC, tweaking the getter wouldn't offer this.  Not sure that matters, but given "manually tweaking" is in the bug title, it kinda makes sense.

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 674462 [details] [diff] [review]
prefs.patch

I agree that this seems like overkill. Changing Social.jsm's Social.enabled getter/setter seems like a simpler fix.

I also don't think we need to go out of our way to support about:config pref flipping.

Comment 16

[:Gavin Sharp [email: [e-mail]]]

(In reply to Mark Hammond (:markh) from comment #14)
> FWIW, the observer means that when manually tweaking via about:config you
> see the effect of this "policy" (ie, manually setting .active to false also
> forces .enabled to false as you change it).  IIUC, tweaking the getter
> wouldn't offer this.  Not sure that matters, but given "manually tweaking"
> is in the bug title, it kinda makes sense.

_setEnabled is called when social.enabled is flipped (the SocialService has its own observer), so this shouldn't be a problem AIUI.

Comment 17

[Shane Caraveo (:mixedpuppy)]

Attachment: prefs.patch

Comment 18

[Shane Caraveo (:mixedpuppy)]

this has all changed drastically since then, doesn't appear to be an issue in current releases.