814404 - [Mac OS] Blank Social menu item in Tools menu when focus is set to a non-browser window

Status: VERIFIED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Virgil Dicu [:virgil] [QA]]

Attachment: Missing menu item-Nightly

Occurs starting with Firefox 17 up to Nightly. only occurs on Mac OS.
 
STR:
1. Start Firefox with clean profile.
2. Open the Download manager (or any other Firefox window)
3. While focus is set to the window opened in step 2, select Tools menu

Actual result: missing menu items for Social API sidebar option (one missing item in Nightly, two in F17,18). See screenshot.

Comment 1

[Virgil Dicu [:virgil] [QA]]

I'll be able to find a regression range for this tomorrow if the culprit isn't found by then.

Comment 2

[Stefan [:stefanh]]

This is not a cocoa bug, more likely a problem with our special handling of the application menubar with its menus/menuitems (macbrowserOverlay, browser-menubar.inc, browser-sets.inc)

Comment 3

[u279076]

Probably a bug with the Social API since it's the Social menu item. More details to follow as I test this further.

Comment 4

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

The code in _providerReady that sets a label on menu_socialAmbientMenu doesn't run in non-browser.xul windows, and so the menu item doesn't get a label there.

We probably need to call some subset of SocialUI.init from nonBrowserWindowDelayedStartup (in addition to the existing call in delayedStartup).

Comment 5

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

This probably dates back to the introduction of this menu item.

Comment 6

[u279076]

Last good nightly: 2012-07-25
First bad nightly: 2012-07-26

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ef20925bc2a5&tochange=7065b767f30d

There is quite a few changes here so I will try to narrow it down further. Just to provide more details about the actual results in testing this...

1. Start Firefox with a new profile
2. Click the Tools menu
> Tools
> -- Web Search
> -- Downloads
> -- Set up Sync...
> -- Web Developer >
> -- Page Info
> -- Start Private Browsing
> -- Clear Recent History
3. Open another window (ex. Bookmarks > Show All Bookmarks)
4. Click the Tools menu
> Tools
> -- Web Search
> -- Downloads
> -- Add-ons
> -- *blank*
> -- Set up Sync...
> -- Web Developer >
> -- Page Info (greyed out)
> -- Start Private Browsing
> -- Clear Recent History
5. Click the *blank* menu item
> Social toolbar and sidebar enabled

Comment 7

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

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #5)
> This probably dates back to the introduction of this menu item.

Yep, that'd be bug 764872 in your range.

Comment 9

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

Attachment: WIP patch

This patch refactors all the updates to the commands and broadcasters to be located in a single place that can be called whenever the UI should be updated.

The |updateCommands| function is a little too long for my liking. Maybe the separate commands/broadcasters can remain in their own function but this function would just call out to those functions.

I'm going to rework this patch to be much simpler so we can land something for it on Aurora/Beta.

Comment 10

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

Attachment: simple patch

This just hides the broken menu items when non-browser windows are focused. This isn't ideal, but it's dead-safe and addresses the most visible bustedness for beta.

Comment 11

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

Comment on attachment 691113 [details] [diff] [review]
simple patch

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

This patch will only work for beta since menu_socialToggle doesn't exist anymore on m-c (aurora too I think).

Comment 12

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

Comment on attachment 691113 [details] [diff] [review]
simple patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): present since socialAPI landed
User impact if declined: ugly empty menu item in Tools menu for non-browser windows
Testing completed (on m-c, etc.): none, only tested locally
Risk to taking this patch (and alternatives if risky): low risk - only hides menu items in non-browser windows. Really can't break anything more than it already is.
String or UUID changes made by this patch: none

Comment 13

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

Attachment: simple patch for aurora and central

I'd like us to land the same simple fix for Aurora and m-c until we get a proper solution implemented, this way we are not under such a tight time crunch to get this fixed.

Comment 14

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

Comment on attachment 691375 [details] [diff] [review]
simple patch for aurora and central

Sounds good! Let's file a followup to fix properly.

Comment 15

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/a050d7a604fe

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a050d7a604fe

Comment 17

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/37af7c9b54f2

Comment 18

[bhavana bajaj [:bajaj]]

Comment on attachment 691113 [details] [diff] [review]
simple patch

Approving for Beta given this is a low risk fix. Virgil, can you please help with some verification on Aurora before we go to build with Beta 5 Tuesday.Thank you !

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d93940399e94

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/d93940399e94

Comment 21

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

Pushed a followup to beta since the wrong patch was initially landed:
https://hg.mozilla.org/releases/mozilla-beta/rev/0a821cf988bb

Comment 22

[Ryan VanderMeulen [:RyanVM]]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #21)
> Pushed a followup to beta since the wrong patch was initially landed:
> https://hg.mozilla.org/releases/mozilla-beta/rev/0a821cf988bb

https://hg.mozilla.org/releases/mozilla-b2g18/rev/0a821cf988bb

Comment 23

[Virgil Dicu [:virgil] [QA]]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:20.0) Gecko/20121217 Firefox/20.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:19.0) Gecko/20121216 Firefox/19.0

Looks good on Mac OS X 10.8 with Aurora and Nightly. No menu entries for Facebook Sidebar when focus is set to another window. Other menu entries are displayed as expected.

Beta verification remaining.

Comment 24

[u279076]

Verified fixed with Firefox 18.0b5.