Closed Bug 273217 Opened 19 years ago Closed 19 years ago

[Firefox] Lots of menu items are enabled that shouldn't be when no windows are open

Categories

(Firefox :: Menus, defect)

PowerPC
macOS
defect
Not set
major

Tracking

()

VERIFIED FIXED
Firefox1.5

People

(Reporter: asaf, Assigned: asaf)

References

Details

(Keywords: platform-parity)

Attachments

(2 files, 2 obsolete files)

firefox version of bug 75660
Target Milestone: --- → Firefox1.1
Status: NEW → ASSIGNED
Blocks: macmeta
Blocks: 227774, 242819
Summary: Lots of menu items are enabled that shouldn't be when no windows are open → [Firefox] Lots of menu items are enabled that shouldn't be when no windows are open
Attached patch v1 (obsolete) — Splinter Review
(including the fix for bug 229040 as well)
Attachment #171654 - Flags: review?(mconnor)
Comment on attachment 171654 [details] [diff] [review]
v1


>+ command="cmd_toogleTaskbar" checked="true" />


<sigh />
Attachment #171654 - Attachment is obsolete: true
Attachment #171654 - Flags: review?(mconnor)
Attached patch getting better (obsolete) — Splinter Review
1) Fix bug 227525 - file -> open file menu item does not work when no windows
open
2) fix typo
Attachment #171690 - Flags: review?(mconnor)
Attached patch more...Splinter Review
Fix bug 249717 (when no windows open the Go->Home menu does nothing) as well
Attachment #171690 - Attachment is obsolete: true
Attachment #171826 - Flags: review?(mconnor)
Attachment #171690 - Flags: review?(mconnor)
Blocks: 249717
Comment on attachment 171826 [details] [diff] [review]
more...

>Index: browser/base/content/browser-menubar.inc
>===================================================================
>RCS file: /cvsroot/mozilla/browser/base/content/browser-menubar.inc,v
>retrieving revision 1.36
>diff -u -p -8 -r1.36 browser-menubar.inc
>--- browser/base/content/browser-menubar.inc	30 Nov 2004 08:22:43 -0000	1.36
>+++ browser/base/content/browser-menubar.inc	20 Jan 2005 01:29:17 -0000

>                 </menu>
>-                <menu label="&charsetMenu.label;" accesskey="&charsetMenu.accesskey;"
>+                <menu id="charsetMenu"
>+                      label="&charsetMenu.label;" accesskey="&charsetMenu.accesskey;"
>                       datasources="rdf:charset-menu" ref="NC:BrowserCharsetMenuRoot"
>                       oncommand="MultiplexHandler(event)" onpopupshowing="CreateMenu('browser');UpdateMenus(event)" onpopupshown="CreateMenu('more-menu');">

lets just fix all of it please.  adding a really short line looks really weird
since we have a really long line, and the giant line really sucks.  Something
like this should be fine.

		<menu id="charsetMenu" label="&charsetMenu.label;"
		      accesskey="&charsetMenu.accesskey;"
oncommand="MultiplexHandler(event)" 
		      datasources="rdf:charset-menu"
ref="NC:BrowserCharsetMenuRoot"
		      onpopupshowing="CreateMenu('browser');UpdateMenus(event)"
		      onpopupshown="CreateMenu('more-menu');">


>+#ifdef XP_MACOSX
>+// The following functions should be used by both hiddenWindow.xul and windows which
>+// don't have menus on platforms other than Mac OS X

This comment didn't make sense the first time I read it.  Double negatives are
bad.  Try the following

// The following functions should be used by both hiddenWindow.xul 
// and any other windows which only have menus on Mac OS X


Plus the viewSidebarMenu id issue from IRC (breaking all sidebar extensions is
a bad thing!) and we should be done here.
Attachment #171826 - Flags: review?(mconnor) → review-
Attached patch patchSplinter Review
Attachment #172017 - Flags: review?(mconnor)
Comment on attachment 172017 [details] [diff] [review]
patch

>@@ -1223,16 +1273,23 @@
>   case "window":
>     OpenBrowserWindow();
>     break;
>   }
> }
> 
> function loadOneOrMoreURIs(aURIString)
> {
>+#ifdef XP_MACOSX
>+  if (window.location.href != getBrowserURL())
>+  {
>+    newWindow = openDialog(getBrowserURL(), "_blank", "all,dialog=no", aURIString);
>+    return;
>+  }
>+#endif
>   var urls = aURIString.split("|");


r=me with a comment here explaining what we're doing.  (i.e. "we're not a
browser window, pass the URI string to a new browser window")
Attachment #172017 - Flags: review?(mconnor) → review+
checked in
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Domo Arigato, Asaf Romano, for doing the job nobody wanted to. Thank you thank you.

I'd love to verify these as soon as a patched nightly comes up (latest is Jan 20).
v Firefox 20050128.
Status: RESOLVED → VERIFIED
*** Bug 290913 has been marked as a duplicate of this bug. ***
*** Bug 292018 has been marked as a duplicate of this bug. ***
Blocks: deermac
What version is this fix in? Bug 292018 was marked as a duplicate of this and
still happens (Firefox 1.06 OS X). Also another similar issue occurs by doing this:

1. Launch Firefox.
2. Open a new browser window. Download something big (optional)
3. Open the downloads window.
4. Close the original browser window.
5. Try to open another browser window while the download window is open.

Mac OS X 10.3.9
(In reply to comment #13)
> What version is this fix in? 

It will be fixed in Firefox 1.5. Security releases (1.0.6 etc..) only get
security fixes.
*** Bug 314608 has been marked as a duplicate of this bug. ***
QA Contact: bugzilla → menus
You need to log in before you can comment on or make changes to this bug.