Closed Bug 321133 Opened 14 years ago Closed 14 years ago

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

Categories

(Calendar :: Sunbird Only, defect, trivial)

x86
Linux
defect
Not set
trivial

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: elreydetodo, Assigned: sipaq)

Details

Attachments

(1 file, 3 obsolete files)

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.

Attached patch patch v1 (obsolete) — Splinter Review
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-
Attached patch Patch v2 (obsolete) — Splinter Review
Attachment #207811 - Attachment is obsolete: true
Attachment #207823 - Flags: first-review?(jminta)
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-
Attached patch Patch v3 (obsolete) — Splinter 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.
Attachment #207823 - Attachment is obsolete: true
Attachment #207827 - Flags: first-review?(jminta)
Attached patch Patch v4Splinter Review
Integrate the comment into the preprocessor instruction.
Attachment #207827 - Attachment is obsolete: true
Attachment #207834 - Flags: first-review?(jminta)
Attachment #207827 - Flags: first-review?(jminta)
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: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.