Closed Bug 804242 Opened 8 years ago Closed 8 years ago

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

Categories

(Firefox Graveyard :: SocialAPI, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: joe, Assigned: mixedpuppy)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
This appears to be happening with recent nightly, and does not happen on beta or aurora.
Duplicate of this bug: 804260
FWIW, I don't have a Tools menu item on Nightly, Aurora, or Beta on Linux.
OS: Mac OS X → All
Hardware: x86 → All
(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?
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.
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.
(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.
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.
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.
Splitting this bug into 2.  This bug is for FF 17 and 18.  Bug 804442 is tracking the different problem happening currently on nightly.
Summary: Tools > Social menu not showing up → Tools > Social menu not showing up on FF 17/18
Summary: Tools > Social menu not showing up on FF 17/18 → Tools > Social menu not showing up on FF 17/18 when prefs are manually flipped out of order
Attached patch prefs.patch (obsolete) — Splinter Review
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.
Attachment #674462 - Flags: review?(mhammond)
Attachment #674462 - Flags: review?(jaws)
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"?
Attachment #674462 - Flags: review?(mhammond) → review+
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.)
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 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.
Attachment #674462 - Flags: review?(jaws) → review-
(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.
Attached patch prefs.patchSplinter Review
Attachment #674462 - Attachment is obsolete: true
Attachment #676224 - Flags: review?(jaws)
Attachment #676224 - Flags: review?(jaws) → review+
Assignee: nobody → mixedpuppy
Status: NEW → ASSIGNED
this has all changed drastically since then, doesn't appear to be an issue in current releases.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.