813436 - Tools > Facebook Messenger menu shown when Social API is disabled

Status: VERIFIED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Cameron McCormack (:heycam)]

I updated my Nightly just now, and I got a Tools > Facebook Messenger menu with a single item in it "Remove from Nightly".  I haven't enabled the Social API, so I think the menu shouldn't be shown at all.

Comment 1

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

I see this on Windows too.

Comment 2

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

It looks alot like http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-sets.inc#120 should have hidden="true"

Comment 3

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

Looks like it was caused by bug 805217

Comment 4

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

Actually, it looks like we might want a new Social:Active command to hide the entire menu, but I'm sure Jaws will have his own thoughts...

Jaws: feel free to assign to me if you think this is the right approach.

Comment 5

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

Yeah, splitting this out to a Social:Active command would provide a nice separation of concerns, and the SocialToolbarButton could use this command as well.

Comment 7

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

Attachment: various ui tweaks

This adds a new broadcaster socialActiveBroadcaster and both the tools menu and toolbar item observes this for the "hidden" attribute.  browser-social.js adds a new pref observer for Social.active and updates this broadcaster accordingly (I did consider a new observer specifically for the active attribute, but that doesn't really seem worth it).  Note that Social.active is now set *before* setting the pref so the observer sees the correct value for Social.active.

A related issue I also rolled into this patch is that if you just disable Facebook (ie, social.enabled=false, social.active=true) the Tools menu continues to show the ambient stuff - this is fixed by calling SocialMenu.populate() as .enabled is toggled and having SocialMenu.populate() check provider.enabled - which will be false when either Social.active or Social.enabled is false.

Comment 9

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

Comment on attachment 683830 [details] [diff] [review]
various ui tweaks

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

::: browser/base/content/browser-social.js
@@ +94,5 @@
> +          this.updateActiveBroadcaster();
> +          this.updateToggleCommand();
> +          // even though the above will hide/show most elements we care about,
> +          // we fall through so elements can actually be removed if !active.
> +        }

I don't see any problems with always doing the |this.updateActiveBroadcaster(); this.updateToggleCommand();| and it can make sure that this code path is always hit when one of the prefs is changed. What do you think?

Comment 10

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f5142a46e4a

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1f5142a46e4a

Comment 13

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

Attachment: rebased for aurora

Same patch as landed on m-c, but rebased to apply on Aurora

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 805217
User impact if declined: Toolbar and Tools menu has social elements even when social is disabled.
Testing completed (on m-c, etc.): Landed on m-c
Risk to taking this patch (and alternatives if risky): Low risk, only social elements impacted
String or UUID changes made by this patch: None

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8be383c89522

Comment 16

[Cornel Ionce [:noni] [Hubs QA]]

Verified as fixed on Firefox 19 beta 5.

Windows 7 - User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0
Mac OS X - User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:19.0) Gecko/20100101 Firefox/19.0
Linux - User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:19.0) Gecko/20100101 Firefox/19.0

Build ID: 20130206083616

Comment 17

[Ada [:adalucinet]]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20100101 Firefox/20.0
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:20.0) Gecko/20100101 Firefox/20.0
User Agent: Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20100101 Firefox/20.0

Verified as fixed on Firefox 20 beta 6 (Build ID: 20130320062118).