Open Bug 525070 Opened 12 years ago Updated 2 years ago

"Reply"/"Reply to All" toggle in composer

Categories

(Thunderbird :: Message Compose Window, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

People

(Reporter: clarkbw, Unassigned, Mentored)

References

Details

(Whiteboard: [has l10n impact][has wip patch][lang=js|xul])

Attachments

(3 files, 6 obsolete files)

+++ This bug was initially created as a clone of Bug #248681 +++

This bug is a spin off of bug 248681 specifically for creating a UI to support the backend changes that might land.  Much of the UI has already been discussed in the original bug.

Specifically these comments hold the latest UI iteration:
bug 248681 comment #49
bug 248681 comment #50

As of the creation of this bug attachment 408456 [details] [diff] [review] has both the backend and UI bits included.
Whiteboard: [has l10n impact][3.next add-on idea]
Duplicate of this bug: 527014
Attached patch WIP for the front-end changes. (obsolete) — Splinter Review
Cause they were lying around, and I thought this might be a good place for them.
Assignee: nobody → bwinton
Status: NEW → ASSIGNED
I've moved the front-end changes into an extension, but I'm finding that there are things that the extension needs from the core code in order to do its job (or at least to do it more efficiently).  These are those changes.

(Sure it's a WIP, but if people have suggestions on how to make them better, I'm all ears.)

Thanks,
Blake.
I was just reminded that in our reply toggle we should continue to use the reply,reply-all key combos that people already use to swap between the toggle modes.  ctrl-r toggles reply and ctrl-shift-r toggles reply all.
Duplicate of this bug: 576259
Duplicate of this bug: 586324
Target Milestone: Thunderbird 11.0 → ---
It would be nice if we landed this in core, but I don't have time to work on it.  Perhaps someone from the community could pick it up…

Thanks,
Blake.
Assignee: bwinton → nobody
Whiteboard: [has l10n impact][3.next add-on idea] → [has l10n impact][has wip patch][mentor=bwinton][lang=js|xul]
To land only the backend changes in the core and do the front end in an extension or all in core?

(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #8)
> It would be nice if we landed this in core, but I don't have time to work on
> it.  Perhaps someone from the community could pick it up…
> 
> Thanks,
> Blake.
I'm not sure why we wouldn't do it all in core.  I moved it to an extension mostly because that's how we were developing new features back in the day.
Ok. I haven't tested the patch(es) but what has to be done? Does it work at the moment? I guess mozmill tests will be needed.
I think a better idea would be to take the extension, and try to port the code into core, and see what bits from the patches are needed.

Also mozmill tests, yeah.  :)
Attached patch Unified patch (obsolete) — Splinter Review
This patch combines/updates the two old patches and seems to work (without the add-on).

Are there any things I should change? Otherwise I would proceed with mozmill tests.
Assignee: nobody → joachim.herb
Attachment #410799 - Attachment is obsolete: true
Attachment #417963 - Attachment is obsolete: true
(In reply to Joachim Herb from comment #13)
> Created attachment 650369 [details] [diff] [review]
> Unified patch
> 
> This patch combines/updates the two old patches and seems to work (without
> the add-on).
> 
> Are there any things I should change? Otherwise I would proceed with mozmill
> tests.

Let's start writing the mozmill tests, and we'll see what bugs they turn up.  :)

(Also, since I wrote a bunch of the code, when you get it reviewed, or ui-reviewed, you should get someone else to do that.  Someone like mconley, say.)

Thanks,
Blake.
There are lots and lots of annoying trailing white spaces in the files to be patched so first remove them.
Attachment #650369 - Attachment is obsolete: true
Cleaner patch then before.
Here are try server builds for the current version of the patch:
https://tbpl.mozilla.org/?tree=Thunderbird-Try&noignore=1&rev=8be5bf71355a
It looks like nothing gets broken by the patch.
Nice Job Joachim ! Any reasons why you didn't ask review yet ?
I guess I could set it to ui-review? 

But there some bugs: 
If in account settings/Copies & Folders "CC these email addresses" is set, the addresses are not show if "Reply" or "Reply All" is chosen (but if the mail is saved as draft, it can be seen, that the addresses were added).

If "BCC these email addresses" is set or a BCC address is added "by hand" in the custom setting, it is removed if switching to Reply/Reply all.

One additional idea for the UI: Would it make sense to display To/CC/BCC in different styles in the Reply/Reply All modes? E.g. CC addresses in italic, and BCC addresses in italic & underlined (like I do it in the two line header mode of the CompactHeader addon)?

I guess before asking for review I will try to fix the BCC problem.
This is still work in progress as one of the test fails (if reply to all is used, the BCC automatic copy address is not added). But I would like to ask to a "principle" review/review-ui. Just to be sure not to running in a completely wrong direction.
Attachment #650507 - Attachment is obsolete: true
Attachment #653079 - Flags: ui-review?(mconley)
Attachment #653079 - Flags: review?(mconley)
Joachim:

Hey - your patch has bit-rotted. Can you re-generate it? I'll begin my code review in the mean time.

-Mike
Comment on attachment 653079 [details] [diff] [review]
Includes first version of mozmill tests

Review of attachment 653079 [details] [diff] [review]:
-----------------------------------------------------------------

Hey Joachim,

Overall, this looks really good! I haven't had a chance to try the patch yet since it's bitrotted, so I'm withholding my ui-r until I can try it out.

Thanks,

-Mike

::: mail/components/compose/content/MsgComposeCommands.js
@@ +1855,5 @@
>    SetComposeWindowTitle();
>    enableEditableFields();
> +
> +  // Send an event to let people know that we've got some fields.
> +  var event = document.createEvent('Events');

We prefer let over var, and we try not to introduce new vars. So can you please change this, and other usages of var to something like:

let event = document.createEvent('Events');

Thanks!

@@ +2446,5 @@
>    var toolbarset = document.getElementById('customToolbars');
>    toolbox.toolbarset = toolbarset;
>  
>    // Prevent resizing the subject and format toolbar over the addressswidget.
> +  document.getElementById("MsgHeadersToolbar").minHeight = document.getElementById("addresses-grid").boxObject.height;

Why was this changed?

I think I prefer the other way, since it's < 80 chars.

::: mail/components/compose/content/addressingWidgetOverlay.js
@@ +215,5 @@
>      // add all of the recipients for this message to the ignore list for spell check
>      addRecipientsToIgnoreList((gCurrentIdentity ? gCurrentIdentity.identityName + ', ' : '') + msgTo + ', ' + msgCC + ', ' + msgBCC);
>  
> +    var replyMode = document.getElementById("replyMode")
> +    replyMode.getElementsByAttribute("value","reply")[0].disabled = msgSenderReply?false:true;

I think I'd prefer:

replyMode.querySelector('[value="reply"]').disabled = !msgSenderReply;
replyMode.querySelector('[value="replyAll"]').disabled = !msgAllReply;
replyMode.querySelector('[value="replyList"]').disabled = !msgListReply;

@@ +228,5 @@
> +function awSwitchReplyMode()
> +{
> +  let replyMode = document.getElementById("replyMode");
> +
> +  let populateAddresses = function(replyModeValue)

Why the extra function here? I don't see it being reused... why not just inline this stuff after line 252?

@@ +253,5 @@
> +    populateAddresses(replyMode.value);
> +  }
> +  // Set sizes on the toolbar so the format toolbar and it's splitter can't over run us
> +  document.getElementById("MsgHeadersToolbar").minHeight = document.getElementById("addresses-grid").boxObject.height;
> +}

> 80 chars

::: mail/test/mozmill/composition/test-reply-toggle.js
@@ +8,5 @@
> +
> +const MODULE_NAME = "test-reply-toggling";
> +
> +const RELATIVE_ROOT = "../shared-modules";
> +const MODULE_REQUIRES = ["folder-display-helpers", "compose-helpers", "window-helpers",

I'd prefer formatting this list like:

const MODULE_REQUIRES = ["folder-display-helpers",
                         "compose-helpers",
                         "window-helpers",
                         "message-helpers"];

@@ +12,5 @@
> +const MODULE_REQUIRES = ["folder-display-helpers", "compose-helpers", "window-helpers",
> +                         "message-helpers"];
> +
> +var elib = {};
> +Cu.import("resource://mozmill/modules/elementslib.js", elib);

You never use elib, so you don't need to import it.

@@ +15,5 @@
> +var elib = {};
> +Cu.import("resource://mozmill/modules/elementslib.js", elib);
> +Cu.import("resource:///modules/mailServices.js");
> +
> +const identity1Email = "carl@example.com";

We like to put k's in front of constants so that we can differentiate them from within our code. So, kIdentity1Email, kIdentity2Email, etc.

@@ +20,5 @@
> +const identity2Email = "lenny@springfield.invalid";
> +const identity2CC = "Automatic.CC@sprintfield.invalid";
> +const identity2BCC = "Automatic.BCC@sprintfield.invalid"
> +
> +var composeHelper = null;

let over var

And we don't need composeHelper.

@@ +21,5 @@
> +const identity2CC = "Automatic.CC@sprintfield.invalid";
> +const identity2BCC = "Automatic.BCC@sprintfield.invalid"
> +
> +var composeHelper = null;
> +var cwc = null; // compose window controller

Unless you're sharing cwc from test to test (which I'd advise against anyhow), I'd say you don't need this global.

@@ +23,5 @@
> +
> +var composeHelper = null;
> +var cwc = null; // compose window controller
> +var folder1;
> +var folder2;

Globals should start with g's, like:

gFolder1
gFolder2

etc

@@ +24,5 @@
> +var composeHelper = null;
> +var cwc = null; // compose window controller
> +var folder1;
> +var folder2;
> +var acctMgr;

We don't need acctMgr.

@@ +26,5 @@
> +var folder1;
> +var folder2;
> +var acctMgr;
> +
> +var setupModule = function (module) {

This can simply be:

function setupModule(module) {
...
}

@@ +27,5 @@
> +var folder2;
> +var acctMgr;
> +
> +var setupModule = function (module) {
> +  let fdh = collector.getModule("folder-display-helpers");

Instead of using fdh, just do:

collector.getModule("folder-display-helpers").installInto(module);

@@ +29,5 @@
> +
> +var setupModule = function (module) {
> +  let fdh = collector.getModule("folder-display-helpers");
> +  fdh.installInto(module);
> +  composeHelper = collector.getModule("compose-helpers");

Same as above - we don't need composeHelper.

collector.getModule("compose-helpers").installInto(module);

@@ +31,5 @@
> +  let fdh = collector.getModule("folder-display-helpers");
> +  fdh.installInto(module);
> +  composeHelper = collector.getModule("compose-helpers");
> +  composeHelper.installInto(module);
> +  let wh = collector.getModule("window-helpers");

Same as above

@@ +33,5 @@
> +  composeHelper = collector.getModule("compose-helpers");
> +  composeHelper.installInto(module);
> +  let wh = collector.getModule("window-helpers");
> +  wh.installInto(module);
> +  let mh = collector.getModule("message-helpers");

Same as above.

@@ +36,5 @@
> +  wh.installInto(module);
> +  let mh = collector.getModule("message-helpers");
> +  mh.installInto(module);
> +
> +  acctMgr = Cc["@mozilla.org/messenger/account-manager;1"]

Use MailServices.accounts instead - and instead of aliasing it to acctMgr, just use it directly.

@@ +41,5 @@
> +    .getService(Ci.nsIMsgAccountManager);
> +
> +  let server1 = acctMgr.createIncomingServer("nobody",
> +                                             "Reply Identity Testing", "pop3");
> +  folder1 = server1.rootFolder.QueryInterface(Ci.nsIMsgLocalMailFolder)

I'd prefer:

gFolder1 = server1.rootFolder
                  .QueryInterface(Ci.nsIMsgLocalMailFolder)
                  .createLocalSubfolder("TestReplyToggling1");

@@ +46,5 @@
> +                   .createLocalSubfolder("TestReplyToggling1");
> +
> +  let server2 = acctMgr.createIncomingServer("nobody2",
> +                                             "Reply Identity Testing 2", "pop3");
> +  folder2 = server2.rootFolder.QueryInterface(Ci.nsIMsgLocalMailFolder)

Same formatting advice as above.

@@ +79,5 @@
> +
> +function reply_selected_message_and_go_to_drafts_folder(f) {
> +  const kText = "Hey check out this megalol link";
> +  // opening a new compose window
> +  cwc = f(mc);

Instead of relying on the global cwc, just pass a controller into this function.

@@ +85,5 @@
> +  cwc.type(cwc.eid("content-frame"), kText);
> +
> +  plan_for_window_close(cwc);
> +  // mwc is modal window controller
> +  plan_for_modal_dialog("commonDialog", function click_save (mwc) {

Formatting is busted here, should be:

plan_for_modal_dialog("commonDialog", function click_save (mwc) {
  //accept saving
  mwc.window.document.documentElement.getButton('accept').doCommand();
});

@@ +94,5 @@
> +  // quit -> do you want to save ?
> +  cwc.window.goDoCommand('cmd_close');
> +  // wait for the modal dialog to return
> +  wait_for_modal_dialog();
> +  // actually quite the window

typo: quite -> quit or close

@@ +132,5 @@
> +    "The TO address " + replyToAddresses.value.join(',') + " of the replied " +
> +    "mail should be the same as the as the original FROM address" +
> +    FromAddresses.value.join(','));
> +
> +  let expectedReplyCCAddresses = [''];

Why not just use:

assert_equals(replyCCAddresses.value.join(','), '', "The CC address...") ?

@@ +273,5 @@
> +    "The TO address " + replyToAddresses.value.join(',') + " of the replied " +
> +    "mail should be the same as the as the original FROM address" +
> +    FromAddresses.value.join(','));
> +
> +  let expectedReplyCCAddresses = ToAddresses.value.concat(CCAddresses.value)

I'd prefer:

let expectedReplyCCAddresses = ToAddresses.value
                                          .concat(CCAddresses.value)
                                          .concat(identity2CC)
                                          .sort();

::: mailnews/test/resources/messageGenerator.js
@@ +557,5 @@
> +  /**
> +   * @returns The comma-ized list of name-and-address tuples used to set the Bcc
> +   *     header.
> +   */
> +  get bcc() { return this._bcc; },

Instead of one-lining it, I'd prefer:

get bcc() {
  return this._bcc;
},

Newline after this getter
Attachment #653079 - Flags: ui-review?(mconley)
Attachment #653079 - Flags: review?(mconley)
Attachment #653079 - Flags: review-
Update patch
Attachment #650506 - Attachment is obsolete: true
Attachment #655652 - Flags: review?(mconley)
This includes the requested changes by the reviewer. Still on mozmill test fails and some test should be added (e.g. toggling the reply mode in the compose window).
Attachment #653079 - Attachment is obsolete: true
Attachment #655752 - Flags: ui-review?(mconley)
Should I ask for check-in of the first part of the patch to avoid bit-rot?
(In reply to Joachim Herb from comment #25)
> Should I ask for check-in of the first part of the patch to avoid bit-rot?

Sure.
Please checkin the first patch: https://bugzilla.mozilla.org/attachment.cgi?id=655652
Keywords: checkin-needed
Comment on attachment 655652 [details] [diff] [review]
First remove all trailing whitespaces in files to be patched

https://hg.mozilla.org/comm-central/rev/1dcadef385b1
Comment on attachment 655752 [details] [diff] [review]
Patch the functionality for reply/reply to all toggle (with requested changes)

Ok, I've taken a look at this. Good first try - but I think there are some improvements to be made in the interface.

I'm not too wild about the dropdown on the left-hand side of the compose window fields for choosing which mode I'm replying in. It's jarring, and seems unbalanced due to the large dropdown for the sender identity above and to the right of it.  How do other email providers, like Gmail, Hotmail, Yahoo mail, etc, solve this?

http://i.imgur.com/T2zMq.png

I also noticed that the "custom" To/Cc/Bcc fields flashed for a second before being hidden. That's also jarring.

There are also some instances where when I reply to a message, there's no address listed next to the reply type dropdown, so I get this big blank, which looks *really* uneven.

See: http://i.imgur.com/LbSX6.png
Attachment #655752 - Flags: ui-review?(mconley) → ui-review-
There are basically a few implementations I am aware of

1) with a dropdown similar to this

It is typically placed as a button somewhere off on a toolbar but buttonizing the Reply: seems OK too

2) with multiple buttons like reply/reply all/..., typically on a toolbar which frob the address.

3) toggle between reply/reply all which shows the state which it would toggle to, not the current state - quite confusing sometimes

To solve the problem with huge blank there should be just an edit which is frobbed with these buttons/dropdowns. As a feedback you could show 'custom' in dropdown or un-depress button once the user edits the field. 

And yes, it should be editable regardless the chosen mode. The reply/reply all should be shortcuts to get predefined recipient list, not an impediment to entering a recipient address.
In google mail there are two buttons above the message. If you press reply, all addresses entered are deleted and only the original sender is inserted. If pressing reply all, the original To/CC/BCC(?) addresses are inserted and all other addresses are removed. If I remember correctly this also the way it is done in Lotus Notes. In Outlook there seems no such feature.

I am not sure that it is a good idea to overwrite (additional) addresses the user has entered. So perhaps there should be a dialog asking before overwriting addresses when switching from "custom" mode to reply/reply toggle mode. But I wouldn't disabled the buttons.

Otherwise I would suggest adding three buttons (reply/reply all/reply list) to the composeToolbar2 which look like the corresponding buttons on the header toolbar and which change between the reply modes (like in the drop down menu bar as in the current version of the patch). 

I am no fan of the "intelligent" reply button in the message header toolbar but a "clone" of it might also be used in composeToolbar2.
This shows how the toolbar of the compose window would look like if the reply/reply all/reply list buttons were added.

What do you think about loosing addition addresses if pressing one of the buttons? A dialog to confirm? Or would be an addition custom reply button be a good idea? (see comment above)
Attachment #658162 - Flags: ui-review?(mconley)
Looks reasonable.

No dialog, please.

It's an edit operation so you should be able to undo with ^Z in the text field. Well, technically. Not sure if the history is preserved when another control gets focus.

Also since there are these nice buttons when you press one it should stay depressed until you press another or edit one of the text fields. And the one that is picked by default should be depressed to start with. That way the user knows if there are changes to lose.
(In reply to Michal 'hramrach' Suchanek from comment #33)
> Looks reasonable.
> 
> No dialog, please.
> 
> It's an edit operation so you should be able to undo with ^Z in the text
> field. Well, technically. Not sure if the history is preserved when another
> control gets focus.
The current patch stores the additional addresses internally, so when switching back to custom reply mode, they are restored. I am not sure that ^Z is an intuitive way to get the addresses back? But I also have no better idea (other than perhaps an custom reply button)

> Also since there are these nice buttons when you press one it should stay
> depressed until you press another or edit one of the text fields. And the
> one that is picked by default should be depressed to start with. That way
> the user knows if there are changes to lose.
Do you know any examples of such depressed buttons in Thunderbird/Firefox? Sounds more like the functionality of radio buttons. But I also think that a depressed button would look better.
(In reply to Joachim Herb from comment #34)
> Do you know any examples of such depressed buttons in Thunderbird/Firefox?
> Sounds more like the functionality of radio buttons. But I also think that a
> depressed button would look better.

The Quick Filter Bar button for example. It uses checked=true.

Why use multiple buttons? What if when in Reply mode then only show the Reply All (or Reply List) button and when in Reply All mode only show the Reply button?
(In reply to Richard Marti [:Paenglab] from comment #35)
> (In reply to Joachim Herb from comment #34)
> > Do you know any examples of such depressed buttons in Thunderbird/Firefox?
> > Sounds more like the functionality of radio buttons. But I also think that a
> > depressed button would look better.
> 
> The Quick Filter Bar button for example. It uses checked=true.
Ok. Got it. Thank you.

> Why use multiple buttons? What if when in Reply mode then only show the
> Reply All (or Reply List) button and when in Reply All mode only show the
> Reply button?
Wouldn't this be looking to nervous? You click on a button (reply) and it disappears and two other buttons (reply all/list) appear?
Do you really need both buttons (reply all/list)? If you are on a mailing list, do you need then reply all? On normal messages with multiple recipients the reply list isn't used.
Probably someone needs it, they are all valid use cases.
I agree hiding buttons due to a click will not work out well, using the depressed state sounds much better.
I agree with Richard, the trend is toward less UI, not more. Stuffing three buttons in the default config just "to be sure" is a unwise decision. Users can always customize the toolbar and add the "Reply to list" button, if they happen to use mailing lists a lot. Most of the millions of Thunderbird users do that not, however.

FWIW, everyone who wants to try the single-button approach should look at Postbox (free trial), which has adopted the same solution for the reply/reply-all problem. (Someone could add this bug here to the "postbox-parity" tracker bug 737347...)
The problem is that to have this feature need some UI for it.

Adding two buttons and hiding third somewhere in preferences so that potential users have to fish it out of there is not a good idea. It's way easier to remove buttons than to add them. Also removing one of the buttons will not give the full picture when the removed one is the default.

The single button approach has less UI but is way more confusing and way less accessible since dropdowns are notoriously poorly implemented.
Who said something about a dropdown? The user already clicked one of "Reply" or "Reply All" to even get into the Compose Window - and then the opposite mode can be activated by a single button in the toolbar. If you're in "Reply" mode, it reads "Reply All", and vice versa. Each time you click it, its label and icon changes - like the Stop/Reload button in Firefox. You never need both at the same time, they are mutually exclusive!
This is what I'd recommend for a mainstream solution... Hardcore mailing list users could add the reply-to-list button, if they need it often. But then I'd expect them to click "Reply to list" from the preview pane in the first place.
This fails in multiple ways.

First you need three buttons which you can't possibly fit as two states of a single button.

Second the stop/reload button is paired with the throbber which shows that the page is loading. This is missing for reply/reply-to/reply-list/custom address (yes, four states). With all three buttons present these all states can be represented in user-intelligible way. With only one button they can't be even reached. 

How you add a reply-to-list to a reply/reply-all toggle in a way to get intelligible UI is a mystery to me. Please explain in detail how would such UI fit together.

Lastly you argue that the user can press the correct button when opening the compose window. This is completely true. Based on this argument and on the argument for minimum UI not even the toggle should be provided because it is superfluous. 

Except all this bug report is about adding such button to the compose window so that you don't have to close and re-open it to press the button. So I guess your argument about pressing the correct button to open the compose window is totally pointless.
Adding my 2¢: This feature is important, but not important enough to warrant multiple buttons. I'd rather have a drop-down button with check boxes or radio buttons. If you use check boxes, "other recipients" and "lists" might be appropriate labels for them. For radio buttons "all" and "lists" would perhaps be better, if "all" includes lists as well.

The button as a whole could be labeled "Reply To" and might one day open some fancy dialog including all the various alternatives discussed in bug #498448. Until then you could turn it into a toggle between "Reply" and "Reply All", the way the summary requests, and see that landed first.
Comment on attachment 655752 [details] [diff] [review]
Patch the functionality for reply/reply to all toggle (with requested changes)

Review of attachment 655752 [details] [diff] [review]:
-----------------------------------------------------------------

I just noticed below that you aren't accounting for NNTP here. We should support that, since it's not too much in the way of overhead.

::: mail/components/compose/content/MsgComposeCommands.js
@@ +2279,5 @@
> +    replyMode.value = "replyAll";
> +  else if (params.type == nsIMsgCompType.ReplyToList)
> +    replyMode.value = "replyList";
> +  else
> +    replyMode.value = "replyCustom";

Don't forget to account for NNTP here (nsIMsgCompType.ReplyToGroup corresponds to the Followup button).
I ran the mozmill tests of the latest patch with the current Thunderbird release (14.0) and also with the current beta (15.0b4) and Earlybird (16.0a2). All produce the same error for the last test case: 
Reply to a mail which included bcc addresses (e.g. from the sent folder, so the bcc addresses were saved) from an account with automatic sending mails to bcc addresses. Then these automatic bcc addresses are not added to the mail, when reply to all is clicked/selected.

So I guess this is a bug in the current implementation of Thunderbird. Should I file an additional bug report for this or should it be covered here?
(In reply to Joachim Herb from comment #45)
> I ran the mozmill tests of the latest patch with the current Thunderbird
> release (14.0) and also with the current beta (15.0b4) and Earlybird
> (16.0a2). All produce the same error for the last test case: 
> Reply to a mail which included bcc addresses (e.g. from the sent folder, so
> the bcc addresses were saved) from an account with automatic sending mails
> to bcc addresses. Then these automatic bcc addresses are not added to the
> mail, when reply to all is clicked/selected.
> 
> So I guess this is a bug in the current implementation of Thunderbird.
> Should I file an additional bug report for this or should it be covered here?

And by switching to another account and then back again, the missing automatic bcc address(es) are added.
Joachim:

I'm so sorry this slipped off the radar! Starting ui-review.

-Mike
Instead of having the three buttons like that, I think I'd personally prefer a menu-button - like the tag button in the 3pane - except that the buttons content reflects the current selection.

Having a button that cycles through individual states, a la Postbox, is confusing...when I click on "Reply All" and it switches to "Reply", did I just choose to switch to Reply All mode now? Or have I switched away to Reply from Reply All? The interface is ambiguous in this regard.
Comment on attachment 658162 [details]
Reply buttons in compose window toolbar

See my previous comment - I think I'd prefer a menu button...though I'm certainly open to argument.
Attachment #658162 - Flags: ui-review?(mconley) → ui-review-
Depends on: 798282
I'd like to be able to press my Reply-All hotkey to change to Reply-All if I accidentally hit Reply. Or vice versa. Apple's Mail.app lets you do this.
(In reply to Matt B from comment #50)
> I'd like to be able to press my Reply-All hotkey to change to Reply-All if I
> accidentally hit Reply. Or vice versa. Apple's Mail.app lets you do this.

(Inside the Compose window, I mean.)
Joachim, could you confirm that you're still working on this?
Flags: needinfo?(joachim.herb)
(In reply to Josh Matthews [:jdm] from comment #52)
> Joachim, could you confirm that you're still working on this?

Sorry, but due to the problems with Bug 798282 I stopped working on this bug: At the moment it is not clear to me how the expected behavior of the CC and BCC field is in the case of a reply to all plus automatic CC/BCC in the preferences. Then Bug 798282 should be fixed first.
Status: ASSIGNED → NEW
Flags: needinfo?(joachim.herb) → needinfo?
Mentor: bwinton
Whiteboard: [has l10n impact][has wip patch][mentor=bwinton][lang=js|xul] → [has l10n impact][has wip patch][lang=js|xul]
Assignee: joachim.herb → nobody
Flags: needinfo?
You need to log in before you can comment on or make changes to this bug.