Closed Bug 321133 Opened 17 years ago Closed 17 years ago
... is on the wrong menu with a wrong name
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:
on firefox, it depends on the OS. Only on linux its edit->preferences. On windows, it is in tools->options.
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.
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.
Assignee: mostafah → bugzilla
Status: UNCONFIRMED → ASSIGNED
Attachment #207811 - Flags: first-review?(jminta)
Let me add that I couldn't test this patch, due to lack of an UNIX system.
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?
Attachment #207811 - Flags: first-review?(jminta) → first-review-
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. :-(
Attachment #207823 - Flags: first-review?(jminta) → first-review-
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.
Integrate the comment into the preprocessor instruction.
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.)
(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 on attachment 207834 [details] [diff] [review] Patch v4 r=jminta given sipaq's last comment
Attachment #207834 - Flags: first-review?(jminta) → first-review+
Patch checked in, leaving bug open for someone to get around to handle the calendar.xpi parts of this.
Bug was left open for handling the XPI part. -> FIXED? (becauce there was a patch for Sunbird)
(In reply to comment #14) > Bug was left open for handling the XPI part. > -> FIXED? (becauce there was a patch for Sunbird) Yes.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.