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

RESOLVED FIXED

Status

--
trivial
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: elreydetodo, Assigned: sipaq)

Tracking

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

13 years ago
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.
(Reporter)

Comment 2

13 years ago
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.

(Assignee)

Comment 3

13 years ago
Created attachment 207811 [details] [diff] [review]
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.
Assignee: mostafah → bugzilla
Status: UNCONFIRMED → ASSIGNED
Attachment #207811 - Flags: first-review?(jminta)
(Assignee)

Comment 4

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

Comment 5

13 years ago
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-
(Assignee)

Comment 6

13 years ago
Created attachment 207823 [details] [diff] [review]
Patch v2
Attachment #207811 - Attachment is obsolete: true
Attachment #207823 - Flags: first-review?(jminta)

Comment 7

13 years ago
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-
(Assignee)

Comment 8

13 years ago
Created attachment 207827 [details] [diff] [review]
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.
Attachment #207823 - Attachment is obsolete: true
Attachment #207827 - Flags: first-review?(jminta)
(Assignee)

Comment 9

13 years ago
Created attachment 207834 [details] [diff] [review]
Patch v4

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 10

13 years ago
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.)
(Assignee)

Comment 11

13 years ago
(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

13 years ago
Comment on attachment 207834 [details] [diff] [review]
Patch v4

r=jminta given sipaq's last comment
Attachment #207834 - Flags: first-review?(jminta) → first-review+

Comment 13

13 years ago
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)
(Assignee)

Comment 15

13 years ago
(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
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.