Last Comment Bug 16908 - [FEATURE]UI: "New Msg" button needs a sub-button (or click and hold), so I can force html or plain text compose
: [FEATURE]UI: "New Msg" button needs a sub-button (or click and hold), so I ca...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Message Display (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: seamonkey2.0b2
Assigned To: Philip Chee
:
Mentors:
: 111311 167588 (view as bug list)
Depends on: 506461
Blocks: HTML-compose-tracker 154188 507871
  Show dependency treegraph
 
Reported: 1999-10-20 13:45 PDT by (not reading, please use seth@sspitzer.org instead)
Modified: 2009-08-04 15:52 PDT (History)
22 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Diff to add a menubutton for New Msg (6.35 KB, patch)
2000-12-09 10:12 PST, Fabian Guisset
no flags Details | Diff | Splinter Review
Updated patch for pulldown menu for New Msg Button. (3.66 KB, patch)
2002-06-20 17:08 PDT, varada
mnyromyr: review-
Details | Diff | Splinter Review
Patch v2.0 Updated to trunk. Fixed Nits. (6.42 KB, patch)
2009-07-28 03:02 PDT, Philip Chee
mnyromyr: review-
Details | Diff | Splinter Review
Patch v2.1 Nits fixed. (6.77 KB, patch)
2009-07-31 03:59 PDT, Philip Chee
mnyromyr: review+
neil: superreview+
Details | Diff | Splinter Review
Patch v2.2 Final Nits fixed [Checkin: Comment 55] (6.82 KB, patch)
2009-08-01 21:04 PDT, Philip Chee
philip.chee: review+
philip.chee: superreview+
Details | Diff | Splinter Review

Description (not reading, please use seth@sspitzer.org instead) 1999-10-20 13:45:36 PDT
in 4.x, on linux and windows, "New Msg" would pop up a sub menu if I clicked and
held the button.

the sub-menu had two items:  "in Plain Text" and "in HTML"

this would allow me to force what style of compose I want.

talked to hangas about this, and he said that there is not going to be "click and
hold" button, but that hyatt has new button, with a sliver or something., that we
should be using.
Comment 1 Phil Peterson 1999-10-21 10:04:59 PDT
I'm not sure about this. 4.x/Windows doesn't have that extra menu, and if we
were going to add one for mozilla, I might rather add a list of identities. We
did have the modifier key which reversed your compose-html preference (e.g. if
your compose-html preference is TRUE and you press Control-NewMessage, you get a
plain text compose window)
Comment 2 Phil Peterson 1999-11-09 17:01:59 PST
Hmm. No comments in here. Does anyone want to debate this before I resolve the
bug wontfix?
Comment 3 lchiang 2000-02-23 22:35:52 PST
perhaps for an issues meeting?
Comment 4 (not reading, please use seth@sspitzer.org instead) 2000-03-31 09:31:25 PST
we can do this now!

ben and hyatt have created the exact widget we need for this.

(I think it's <menulist>, but I'll go check)
Comment 5 selmer (gone) 2000-04-03 18:47:16 PDT
Syncing with marketing priorities.  These are moving to P4 to connote "Cut here
first".  If you disagree with this priority, please let me know.
Comment 6 lchiang 2000-04-04 16:04:25 PDT
move to M16.  Not M15 stoppers
Comment 7 (not reading, please use seth@sspitzer.org instead) 2000-04-11 14:51:55 PDT
er, menulist is not the right widget, but I think the right one for this exists.
Comment 8 Ben Goodger (use ben at mozilla dot org for email) 2000-04-21 23:58:30 PDT
menubutton is what you're looking for...
Comment 9 (not reading, please use seth@sspitzer.org instead) 2000-04-22 00:28:59 PDT
ah, like the back forward button in the browser.  yes, that is what I need.
Comment 10 scottputterman 2000-04-23 23:37:36 PDT
are we going to do this on reply and reply-all as well?  If so, are we going to 
have this for each of the choices for these buttons?
Comment 11 (not reading, please use seth@sspitzer.org instead) 2000-04-24 13:05:50 PDT
looking at the spec, it doesn't look like any of these "menu buttons" are 
mentioned.

but I'd like to see them as 4.x had them.  

jglick, comments?
Comment 12 german 2000-04-24 13:20:30 PDT
Sorry we didn't get around to fully spec'em yet, but for usability reasons we 
have slightly added to the behavior compared to 4.x by having a seperate XUL area 
to the right of the button that expands the additional menu/options. By providing 
a larger target area as well as more discoverable behavior we hope to remedy the 
usability problems that plagued the 4.x implementation. As Ben pointed out the 
proper widget to use is menu button.
Comment 13 german 2000-04-24 13:21:34 PDT
I wanted to add that this design replaces the time-out based behavior for popping 
up menus from toolbar buttons in 4.x.
Comment 14 scottputterman 2000-05-01 16:13:54 PDT
I don't think we need to do this for M16.  It's not really much of a feature
since we already have code that makes hitting shift-button click do what Seth
wants. So it's just a matter of adding xul which shouldn't be that hard.  I
suggest that we can move this out.
Comment 15 selmer (gone) 2000-05-08 12:41:51 PDT
Marking M20.
Comment 16 Fabian Guisset 2000-12-09 10:12:04 PST
Created attachment 20412 [details] [diff] [review]
Diff to add a menubutton for New Msg
Comment 17 Fabian Guisset 2000-12-09 10:17:36 PST
This diff
1) Turns the New Msg button into a menubutton with two menuitems (HTML and Plain 
Text) --> File mailWindowOverlay.xul
2) Adds the two needed js functions --> File mailWindowOverlay.js
3) Adds the two dtd entities --> File messenger.dtd
4) Turns the css for the button into a css for the menubutton (copied from the 
print menubutton) for the modern skin.
5) Turns the css for the button into a css for the menubutton (copied from the 
print menubutton) for the classic skin.

N.B : I'm not a skin master, so the css changes need to be reviewed BADLY, 
mainly for the classic skin.
(Don't have cvs access to make a real patch, sorry) Tested successfully by 
walk84@usa.net and myself (hidday@geocities.com).

Please give your input, thank you,
Fabian.
Comment 18 Matthew Paul Thomas 2000-12-10 07:41:24 PST
Suggest wontfix. If you make it a feature of the `New' button, then it should 
really be a feature of the `Reply' and `Reply All' buttons as well. But they're 
already going to have pulldown menus for reply to sender/newsgroup/both (bug 
17796).

I can't believe that anyone is going to switch composition formats often enough 
that they need visible UI in the toolbar to do it. A modifier key (Shift+click) 
would be quite enough.
Comment 19 Fabian Guisset 2000-12-10 09:06:14 PST
Thanx for saying this 6 monthes later. 
For your information shift-click already switches between default and opposite 
of default (in the account pref), but (almost) nobody knows of this. Quite a few 
people asked for this as well as for the reply/reply all and for the forward and 
for the next button (bug 59264).
Need Info : what is the difference between a pull-down menu and a menubutton?
N.B : I didn't ask for this, mail folks did, not like I use extremely often.
Fabian.
Comment 20 wkfx2003-bugzilla 2001-08-16 21:03:18 PDT
I agree that nobody knows the modifier key of "New Msg". It is really news to
me. I just know it today when I come across this bug!!

But I do hope we have a drop down for New Msg to chooce identity like Outlook
Express does. Refer to the second comment (from Phil Peterson).
Comment 21 Jean-Francois Ducarroz 2001-10-03 17:01:41 PDT
reassign to varada
Comment 22 Mark Renouf 2001-10-24 11:17:43 PDT
The "Shift-Click->New Message" trick works well, and it's exactly what I was
looking for when searching for related bugs. Can't this just be made a pref for
now? 

I see the options for specifying the format *after* the message is sent, but
what if I want to send in plaintext to start with... by default? There is
nowhere in the prefs to specify your default message editing format... 

Add a simple pulldown to choose under Preferences|Mail&Newsgroups|Message
Composition  ?

Or maybe under account settings, one setting per each account. Maybe I want HTML
for email at work, plaintext for email at home, and plaintext for news posts.
Comment 23 Alex Bishop 2001-12-17 15:31:00 PST
*** Bug 111311 has been marked as a duplicate of this bug. ***
Comment 24 Alex Bishop 2001-12-17 15:35:26 PST
> Or maybe under account settings, one setting per each account. Maybe I want HTML
> for email at work, plaintext for email at home, and plaintext for news posts.

In Edit > Mail & Newsgroups Account Settings, there's a per account 'Compose
Messages in HTML Format' checkbox that fit the bill.
Comment 25 mozbug1 2001-12-17 15:45:08 PST
It is too cumbersome to change that setting to compose one email a different way.
Comment 26 varada 2002-06-20 17:08:28 PDT
Created attachment 88557 [details] [diff] [review]
Updated patch for pulldown menu for New Msg Button.

Updated patch for pulldown menu for New Msg Button.
Comment 27 Jean-Francois Ducarroz 2002-06-24 12:11:21 PDT
Comment on attachment 88557 [details] [diff] [review]
Updated patch for pulldown menu for New Msg Button.

Instead of having two JS functions (MsgNewMessage & NewMessage) to create a new
message window, you should consolidate them into one. Also, did you check with
jeniffer if it was ok to do this UI change? and what about reply and forward
button, do we want also to be able to select the mode?
Comment 28 (not reading, please use seth@sspitzer.org instead) 2002-12-16 12:49:51 PST
taking all of varada's bugs.
Comment 29 johann.petrak@gmail.com 2003-02-13 23:54:20 PST
I think to have an optional drop-down menu for the compose, reply, and forward
buttons (like for the forward/back buttons) to choose between plaintext and html
would be user-friendly, since hardly a user is aware of the shift-click method.
It would not clutter the UI but give a hint that this feature exists. It will
*not* clash with the identities list, since this is taken care of through the
"from" menu in the compose window, no?
There is a potential clash of conflict as some users might want the forward
button to dynamically offer the option to choose between inline and attachment.

So if *someone* (I am not mentioning names here) would come up with a patch,
would it have a chance to get checked in from th UI design POV?
Comment 30 Ralf Hauser 2003-05-14 21:16:37 PDT
http://bugzilla.mozilla.org/show_bug.cgi?id=167588 is very similar
Comment 31 neil@parkwaycc.co.uk 2003-05-15 06:12:28 PDT
*** Bug 167588 has been marked as a duplicate of this bug. ***
Comment 32 Vidar Haarr (not reading bugmail) 2003-08-23 04:37:35 PDT
Suggest WONTFIX.
I don't see the problem. Always use plain text :)
No, seriously, I agree with mpt in comment 18.
Comment 33 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2003-08-23 05:58:17 PDT
There already is an RFE (bug 140800) to dynamically switch between
plaintext/HTML compose when you're in the message compose window. That's the way
Outlook Express does it, and I personally find that very helpful. It would also
remove the need to add extra UI to the Compose/Reply/Forward buttons, which are
overloaded enough.

I suggest WONTFIXing this bug, and focussing on bug 140800.
Comment 34 Robert Kaiser 2009-06-14 06:54:03 PDT
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614
Comment 35 Robert Kaiser 2009-06-14 06:54:48 PDT
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614
Comment 36 Robert Kaiser 2009-06-14 06:59:45 PDT
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614
Comment 37 Ashley Bischoff (blog at handcoding.com) 2009-06-14 08:22:21 PDT
(In reply to comment #36)
> If you can confirm that this report still applies to current SeaMonkey 2.x
> nightly builds, please set it back to the NEW state along with a comment on how
> you reproduced it on what Build ID, or if it's an enhancement request, why it's

Still present in:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090614 Shredder/3.0b3pre

Marking NEW.
Comment 38 Robert Kaiser 2009-06-14 11:19:28 PDT
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614
Comment 39 Robert Kaiser 2009-06-14 13:37:09 PDT
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614
Comment 40 Ashley Bischoff (blog at handcoding.com) 2009-06-14 13:50:51 PDT
(In reply to comment #39)
> If you can confirm that this report still applies to current SeaMonkey 2.x
> nightly builds, please set it back to the NEW state along with a comment on how
> you reproduced it on what Build ID, or if it's an enhancement request, why it's
> still worth implementing and in what way.

Still present in:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre)
Gecko/20090614 Shredder/3.0b3pre

Marking NEW.
Comment 41 Robert Kaiser 2009-06-14 13:53:44 PDT
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614
Comment 42 Robert Kaiser 2009-06-14 13:59:18 PDT
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614
Comment 43 Ashley Bischoff (blog at handcoding.com) 2009-06-15 14:38:06 PDT
Still present in:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre)
Gecko/20090614 Shredder/3.0b3pre

Marking NEW. Again.
Comment 44 Robert Kaiser 2009-06-27 16:48:31 PDT
Comment on attachment 88557 [details] [diff] [review]
Updated patch for pulldown menu for New Msg Button.

patches without a review request are useless, directing to SeaMonkey MailNews owner.
Comment 45 Karsten Düsterloh 2009-07-09 15:17:02 PDT
Comment on attachment 88557 [details] [diff] [review]
Updated patch for pulldown menu for New Msg Button.

First of all, this patch doesn't apply anymore. MailNews may be a slow river, but not _that_ ropy... ;-)

Some structural nits, nonetheless:

>Index: content/mailWindowOverlay.js
>===================================================================
>+function NewMessage(format)

The new functionality should be integrated into the old MsgNewMessage function.
Furthermore, for extended bonus points, the menuitem of the current default setting should be marked with default=true.

Philip, maybe you're interested? O:-)
Comment 46 Philip Chee 2009-07-24 07:13:21 PDT
Taking bug.
Comment 47 Robert Kaiser 2009-07-24 11:56:31 PDT
Note that Lightning already changes that button to a menubutton, I hope that won't be broken by a patch for this.
Comment 48 Philip Chee 2009-07-28 03:02:32 PDT
Created attachment 391045 [details] [diff] [review]
Patch v2.0 Updated to trunk. Fixed Nits.

Updated patch to trunk. Plus ported the relevant bits from Thunderbird Bug 416666 (Clean up Thunderbird's global scope a bit).
Comment 49 Philip Chee 2009-07-28 03:06:05 PDT
> +function ComposeMsgByType(aCompType, aEvent) {

Thunderbird uses camelcase i.e. composeMsgByType() but since this is an internal helper function I understand that the MailNews peers don't care about Thunderbird compatibility here.
Comment 50 Karsten Düsterloh 2009-07-30 14:12:06 PDT
Comment on attachment 391045 [details] [diff] [review]
Patch v2.0 Updated to trunk. Fixed Nits.

No functional problems, just some stylish nitpicking...

>diff --git a/suite/locales/en-US/chrome/mailnews/messenger.dtd b/suite/locales/en-US/chrome/mailnews/messenger.dtd
>+<!ENTITY newPlainTextMessageCmd.label "Compose in PlainText">

All other places in MailNews I found are using the spelling "Plain Text", so this menuitem should do as well.

>diff --git a/suite/mailnews/mailWindowOverlay.js b/suite/mailnews/mailWindowOverlay.js
>+/**
>+ * Calls the ComposeMessage function with the desired type, and proper default

Are you sure about the comma?

>+function ComposeMsgByType(aCompType, aEvent) {

Please use curly-braces-on-their-own-line style.

>+  var format = (aEvent && aEvent.shiftKey) ?
>+               msgComposeFormat.OppositeOfDefault :
>+               msgComposeFormat.Default;

I'd say it's permissible to ignore the 80 column limit here.
Or drag OppositeOfDefault behind the ? and align the : under the ?, with Default following.

> function MsgNewMessage(event)

Should be aEvent since you're rewriting the function anyway.

>+  var mode = event ? event.target.getAttribute("mode") : null;

var mode = event && event.target.getAttribute("mode");

> function MsgReplySender(event)
> function MsgReplyGroup(event)
> function MsgReplyToAllRecipients(event)
> function MsgReplyToSenderAndGroup(event)

Should be aEvent since you're rewriting these functions anyway.
Comment 51 Philip Chee 2009-07-31 03:59:19 PDT
Created attachment 391846 [details] [diff] [review]
Patch v2.1 Nits fixed.

>>diff --git a/suite/locales/en-US/chrome/mailnews/messenger.dtd b/suite/locales/en-US/chrome/mailnews/messenger.dtd
>>+<!ENTITY newPlainTextMessageCmd.label "Compose in PlainText">
> 
> All other places in MailNews I found are using the spelling "Plain Text", so
> this menuitem should do as well.
Fixed.

>>diff --git a/suite/mailnews/mailWindowOverlay.js b/suite/mailnews/mailWindowOverlay.js
>>+/**
>>+ * Calls the ComposeMessage function with the desired type, and proper default
> 
> Are you sure about the comma?
Removed.

>>+function ComposeMsgByType(aCompType, aEvent) {
> 
> Please use curly-braces-on-their-own-line style.
Fixed.

>>+  var format = (aEvent && aEvent.shiftKey) ?
>>+               msgComposeFormat.OppositeOfDefault :
>>+               msgComposeFormat.Default;
> 
> I'd say it's permissible to ignore the 80 column limit here.
> Or drag OppositeOfDefault behind the ? and align the : under the ?, with
> Default following.
Since there are some really long lines already in the file I've put this all on one line.

>> function MsgNewMessage(event)
> 
> Should be aEvent since you're rewriting the function anyway.
Fixed.

>>+  var mode = event ? event.target.getAttribute("mode") : null;
> 
> var mode = event && event.target.getAttribute("mode");
Fixed.

>> function MsgReplySender(event)
>> function MsgReplyGroup(event)
>> function MsgReplyToAllRecipients(event)
>> function MsgReplyToSenderAndGroup(event)
> 
> Should be aEvent since you're rewriting these functions anyway.
Fixed.
Comment 52 Karsten Düsterloh 2009-08-01 05:31:41 PDT
Comment on attachment 391846 [details] [diff] [review]
Patch v2.1 Nits fixed.

Cool. :)
Comment 53 neil@parkwaycc.co.uk 2009-08-01 12:25:35 PDT
Comment on attachment 391846 [details] [diff] [review]
Patch v2.1 Nits fixed.

>+               
Nit: line isn't really empty ;-)

>+function MsgNewMessage(aEvent)
>+{
>+  var mode = aEvent && aEvent.target.getAttribute("mode");
>+  if (mode)
>+    ComposeMessage(msgComposeType.New,
>+                   msgComposeFormat[mode],
>+                   GetFirstSelectedMsgFolder(),
>+                   GetSelectedMessages());
>+  else
>+    ComposeMsgByType(msgComposeType.New, aEvent);
>+}
I think this is confusing. Rather than splitting the work between ComposeMessage and ComposeMsgByType I would just compute the appropriate format in all possible cases and call ComposeMessge directly.

>+        <menuitem label="&newHTMLMessageCmd.label;"
>+                  accesskey="&newHTMLMessageCmd.accesskey;"
>+                  mode="HTML"/>
>+        <menuitem label="&newPlainTextMessageCmd.label;"
>+                  accesskey="&newPlainTextMessageCmd.accesskey;"
>+                  mode="PlainText"/>
Nit: Followup needed to "highlight" the default action. (All the other menubuttons do; Get Messages seems to be the the oddity, but then it applies to the current folder while the menu actually lists accounts.)
Comment 54 Philip Chee 2009-08-01 21:04:50 PDT
Created attachment 392128 [details] [diff] [review]
Patch v2.2 Final Nits fixed
[Checkin: Comment 55]

>(From update of attachment 391846 [details] [diff] [review])
>>+               
> Nit: line isn't really empty ;-)
Fixed.

>>+function MsgNewMessage(aEvent)
>>+{
>>+  var mode = aEvent && aEvent.target.getAttribute("mode");
>>+  if (mode)
>>+    ComposeMessage(msgComposeType.New,
>>+                   msgComposeFormat[mode],
>>+                   GetFirstSelectedMsgFolder(),
>>+                   GetSelectedMessages());
>>+  else
>>+    ComposeMsgByType(msgComposeType.New, aEvent);
>>+}
> I think this is confusing. Rather than splitting the work between
> ComposeMessage and ComposeMsgByType I would just compute the appropriate format
> in all possible cases and call ComposeMessge directly.
Fixed.

>>+        <menuitem label="&newHTMLMessageCmd.label;"
>>+                  accesskey="&newHTMLMessageCmd.accesskey;"
>>+                  mode="HTML"/>
>>+        <menuitem label="&newPlainTextMessageCmd.label;"
>>+                  accesskey="&newPlainTextMessageCmd.accesskey;"
>>+                  mode="PlainText"/>
> Nit: Followup needed to "highlight" the default action. (All the other
> menubuttons do; Get Messages seems to be the the oddity, but then it applies to
> the current folder while the menu actually lists accounts.)

Filed Bug 507871
Comment 55 Serge Gautherie (:sgautherie) 2009-08-04 08:19:19 PDT
Comment on attachment 392128 [details] [diff] [review]
Patch v2.2 Final Nits fixed
[Checkin: Comment 55]


http://hg.mozilla.org/comm-central/rev/a781976a9815
Comment 56 AlexIhrig 2009-08-04 15:52:06 PDT
Is there a Thunderbird equivalent?

Note You need to log in before you can comment on or make changes to this bug.