Closed Bug 814404 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox Graveyard :: SocialAPI, defect)

17 Branch
All
macOS
defect
Not set

Tracking

(firefox17 wontfix, firefox18+ verified, firefox19+ verified, firefox20 verified, b2g18 fixed)

VERIFIED FIXED
Firefox 20
Tracking Status
firefox17 --- wontfix
firefox18 + verified
firefox19 + verified
firefox20 --- verified
b2g18 --- fixed

People

(Reporter: virgil.dicu, Assigned: Gavin)

References

Details

(Whiteboard: [testday-20121221])

Attachments

(4 files)

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.
I'll be able to find a regression range for this tomorrow if the culprit isn't found by then.
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)
Probably a bug with the Social API since it's the Social menu item. More details to follow as I test this further.
Component: Widget: Cocoa → SocialAPI
Product: Core → Firefox
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).
This probably dates back to the introduction of this menu item.
Summary: [Mac OS] Missing menu items from Tools menu when focus is set to another Firefox window → [Mac OS] Missing Social menu item from Tools menu when focus is set to another Firefox window
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
(In reply to :Gavin Sharp (use gavin@gavinsharp.com 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.
Depends on: 764872
Duplicate of this bug: 816438
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Attached patch WIP patchSplinter Review
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.
Assignee: jaws → gavin.sharp
Attached patch simple patchSplinter Review
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.
Attachment #691113 - Flags: review?(jaws)
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).
Attachment #691113 - Flags: review?(jaws) → review+
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
Attachment #691113 - Flags: approval-mozilla-beta?
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.
Attachment #691375 - Flags: review?(gavin.sharp)
Comment on attachment 691375 [details] [diff] [review]
simple patch for aurora and central

Sounds good! Let's file a followup to fix properly.
Attachment #691375 - Flags: review?(gavin.sharp)
Attachment #691375 - Flags: review+
Attachment #691375 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/integration/mozilla-inbound/rev/a050d7a604fe
Hardware: x86 → All
Summary: [Mac OS] Missing Social menu item from Tools menu when focus is set to another Firefox window → [Mac OS] Blank Social menu item in Tools menu when focus is set to a non-browser window
https://hg.mozilla.org/mozilla-central/rev/a050d7a604fe
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
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 !
Attachment #691113 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Contact: virgil.dicu
Pushed a followup to beta since the wrong patch was initially landed:
https://hg.mozilla.org/releases/mozilla-beta/rev/0a821cf988bb
(In reply to :Gavin Sharp (use gavin@gavinsharp.com 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
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.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Verified fixed with Firefox 18.0b5.
Whiteboard: [testday-20121221]
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.