321133 - Options... is on the wrong menu with a wrong name

Status: RESOLVED FIXED

(Calendar :: Sunbird Only, defect)

Description

[Martin Meyer]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20051220 Firefox/1.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20051221 Mozilla Sunbird/0.3a1+

The Options... item under the Tools menu should be called Preferences and appear in the Edit menu.  This is where it is found in both Firefox and Thunderbird and it should be consistent.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Michiel van Leeuwen (email: mvl+moz@)]

on firefox, it depends on the OS. Only on linux its edit->preferences. On windows, it is in tools->options.

Comment 2

[Martin Meyer]

Firefox implements this with compiler directives and different localizations strings for various platforms in browser/base/content/browser-menubar.inc:

@ line 126
126 #ifdef XP_UNIX
127 #ifndef XP_MACOSX
128                 <menuseparator/>
129                 <menuitem id="menu_preferences"
130                           label="&preferencesCmdUnix.label;"
131                           accesskey="&preferencesCmdUnix.accesskey;"
132                           oncommand="openPreferences();"/>
133 #endif
134 #endif

and there's another one later for *not* XP_UNIX under tools.  I don't actually see anything in here to handle OSX but I'm sure it's there somewhere.

The way to fix this would be:
in calendar/resources/locale/en-US/menuOverlay.dtd
after lines 222 and 223 (which defines preferencesCmd) insert:
<!ENTITY preferencesCmdUnix.label                   "Preferences">
<!ENTITY preferencesCmdUnix.accesskey               "n">

This will add a Unix string that will be the same as Firefox's.

in calendar/resources/content/menuOverlay.xul after line 216 add that code I pasted above from Firefox and then add #ifndef XP_UNIX and #endif around lines 213-216.  Note that this still leaves no options dialog for OSX.  I'm not sure what Firefox did with that code but it seems to be left out somehow.  I must be there somewhere though.

Comment 3

[Simon Paquet [:sipaq]]

Attachment: patch v1

This patch implements Edit->Preferences instead of Tools->Options for Sunbird on UNIX platforms with the exception of Mac OS X. 
It also removes an obsolete comment from calendar-menubar.inc, which became obsolete with the checkin for bug 308995 in September 2005.

Comment 4

[Simon Paquet [:sipaq]]

Let me add that I couldn't test this patch, due to lack of an UNIX system.

Comment 5

[Joey Minta]

Comment on attachment 207811 [details] [diff] [review]
patch v1

Won't this give Linux 2 different ways to open preferences, rather than actually moving it?

Comment 6

[Simon Paquet [:sipaq]]

Attachment: Patch v2

Comment 7

[Joey Minta]

Comment on attachment 207823 [details] [diff] [review]
Patch v2

+#ifdef XP_UNIX
+#ifndef XP_MACOSX

Macs don't get in here.

+#ifndef XP_UNIX 
Macs don't get in here either. :-(

Comment 8

[Simon Paquet [:sipaq]]

Attachment: Patch v3

New patch which doesn't leave out Mac OS X.
I added a comment for the Mac OS X section explaining why it's necessary.

Comment 9

[Simon Paquet [:sipaq]]

Attachment: Patch v4

Integrate the comment into the preprocessor instruction.

Comment 10

[Joey Minta]

Comment on attachment 207834 [details] [diff] [review]
Patch v4

Everything here looks good.  Can someone give me an argument for or against making similar changes to the calendar extension?  (If we want to change the extension's menus as well, we should do it all at once, otherwise it will likely fall off the radar, since none of the main contributors work on it much.)

Comment 11

[Simon Paquet [:sipaq]]

(In reply to comment #10)
> Everything here looks good.  Can someone give me an argument for or against
> making similar changes to the calendar extension?

We would have to apply additional #ifdefs for distinguishing Thunderbird and Firefox from Seamonkey for that. I don't think that this is a good idea.

Comment 12

[Joey Minta]

Comment on attachment 207834 [details] [diff] [review]
Patch v4

r=jminta given sipaq's last comment

Comment 13

[Joey Minta]

Patch checked in, leaving bug open for someone to get around to handle the calendar.xpi parts of this.

Comment 14

[Martin Schröder [:mschroeder]]

Bug was left open for handling the XPI part.
-> FIXED? (becauce there was a patch for Sunbird)

Comment 15

[Simon Paquet [:sipaq]]

(In reply to comment #14)
> Bug was left open for handling the XPI part.
> -> FIXED? (becauce there was a patch for Sunbird)

Yes.