Firefox app menu contains only "Quit" in certain edgecases (e.g. starting jsconsole before the main window)

RESOLVED FIXED in Firefox 43



4 years ago
4 years ago


(Reporter: Gijs, Assigned: smichaud)



38 Branch
Firefox 43
Dependency tree / graph

Firefox Tracking Flags

(firefox43 fixed)



(1 attachment, 1 obsolete attachment)



4 years ago
+++ This bug was initially created as a clone of Bug #1151345 +++

From bug 1151345 comment 136:

(In reply to Dan Stillman from comment #136)
> I can still reproduce this consistently in Nightly in a new empty profile
> with the following command line:
> /Applications/ -p "Test" -jsconsole
> -purgecaches
> One difference seems to be that, with -purgecaches, the Browser Console
> opens first, whereas without it the browser window opens first.
> I've also gotten it to occur intermittently in other situations without
> -purgecaches (but still with -jsconsole) where the Browser Console opening
> first isn't predictive.
> (We're also seeing it with standard launching in Zotero Standalone, using
> Firefox 39 as XULRunner, but since that's a more complicated situation I'll
> hold off on that until we debug this in Firefox itself. But started there
> with recent versions as well.)

I can reproduce this.
Flags: needinfo?(smichaud)


4 years ago
> /Applications/ -p "[existingprofile]" -jsconsole

This is all I need to reproduce this bug in today's mozilla-central nightly ... but only with e10s off.  With e10s on I need to add -purgecaches.

Thanks for the easy STR!

Yes, we do need to deal with these edgecases at some point, but for now I'm busy with more urgent bugs.  I hope to find time to get back to this bug in the next few weeks.

Gijs and others, please dup similar bugs to this one.
Flags: needinfo?(smichaud)
Duplicate of this bug: 1177197

Comment 3

4 years ago
My understanding is that the problem here is that Firefox constructs the application menu based on the first window to be opened and never changes it after that. Prior to bug 1127205 Firefox didn't try to construct the application menu until a window with a menubar tag was opened, but now it does. With Dan's STR the first window to open is the JS console, which has no menubar and gets the fallback application menu.

Would it be sufficient just to replace the fallback application menu once a window with a menubar is opened? Or should this code be refactored so that the application menu actually changes when switching between windows?
Simon, what you say is reasonable.  I'll keep it in mind when I get back to this.

Or would you like to work on it yourself? :-)


4 years ago
Depends on: 1186600
Duplicate of this bug: 1195049
Posted patch Fix (obsolete) — Splinter Review
After testing a bit, I believe Simon's comment #3 gets to the heart of the matter.

> Would it be sufficient just to replace the fallback application menu once
> a window with a menubar is opened?

That's what this patch does.  It seems to do the trick in my limited testing.  (It also backs out part of the patch for bug 1151345.)

I've started a set of tryserver builds.  When they're done I'll ask the people who still see this bug to test with them.
Assignee: nobody → smichaud
Attachment #8651262 - Flags: review?(spohl.mozilla.bugs)
Comment on attachment 8651262 [details] [diff] [review]

Review of attachment 8651262 [details] [diff] [review]:

::: widget/cocoa/
@@ +32,5 @@
>  NativeMenuItemTarget* nsMenuBarX::sNativeEventTarget = nil;
>  nsMenuBarX* nsMenuBarX::sLastGeckoMenuBarPainted = nullptr; // Weak
>  nsMenuBarX* nsMenuBarX::sCurrentPaintDelayedMenuBar = nullptr; // Weak
>  NSMenu* sApplicationMenu = nil;
> +BOOL sApplicationMenuIsFallback = false;


@@ +186,5 @@
>                                                    keyEquivalent:keyStr] autorelease];
>    [quitMenuItem setTarget:nsMenuBarX::sNativeEventTarget];
>    [quitMenuItem setTag:eCommand_ID_Quit];
>    [sApplicationMenu addItem:quitMenuItem];
> +  sApplicationMenuIsFallback = true;


@@ +210,5 @@
> +  // If we've created only yet created a fallback global Application menu
> +  // (using ContructFallbackNativeMenus()), destroy it before recreating it
> +  // properly.

The beginning of this comment should be rephrased. Maybe:
"If we've only been able to create a fallback global Application menu previously (using ContructFallbackNativeMenus()), destroy it before recreating it properly."

@@ +513,5 @@
>  {
>    [sApplicationMenu removeAllItems];
>    [sApplicationMenu release];
>    sApplicationMenu = nil;
> +  sApplicationMenuIsFallback = false;

Attachment #8651262 - Flags: review?(spohl.mozilla.bugs) → review+
Carrying over r+.

> s/false/NO/
> s/true/YES/

Argh :-(

> The beginning of this comment should be rephrased.

Argh again :-(  I left in an extra word.  I removed it in what I landed.
Attachment #8651820 - Flags: review+
Attachment #8651262 - Attachment is obsolete: true
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43


4 years ago
Blocks: 1198091
Whoever still saw this bug after the patch for bug 1151345 landed, please test with current mozilla-central nightlies.  Starting with today's m-c nightly, they contain the patch for this bug.

Comment 12

4 years ago
It's working now for me. Thanks.
You need to log in before you can comment on or make changes to this bug.