526303 - (Persistent) "Reply to sender" button on header should not be dropdown

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, defect)

Description

[Thomas D. (:thomas8)]

On new header pane toolbar, with "Always show Reply to Sender" enabled, we show a separate "Reply to sender" button next to the intelligent "Reply all/list/whatever" dropdown.

Actual
- (Persistent) Reply to sender button is a dropdown

Expected
- (Persistent) Reply to sender button should not be dropdown, because it never shows more than 1 item, namely "reply" itself
- it should just be a plain button

Bryan (who implemented persistent reply button, thanks!), you think you could fix this? Having a functionless second dropdown is actually quite odd and confusing.

Comment 1

[Thomas D. (:thomas8)]

This is a minor (partial/incomplete) followup from Bug 511924.

Comment 2

[Jim Porter (:squib)]

Attachment: Fix this

This has been annoying me for ages, and I finally got annoyed enough to fix it. It's a bit more complicated than "make the reply button a regular button", since news shows a "reply" button that has "reply all" as a sub-option, but it's still pretty simple.

I hesitate to write tests for this right now though, since I think I'll be stepping on herb's toes over in bug 519956 if I do so. Nevertheless, tests should be pretty straightforward.

Comment 3

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 541987 [details] [diff] [review]
Fix this

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

I mostly like this patch.  r=me with the nits below fixed.

::: mail/base/content/mailWindowOverlay.js
@@ +996,5 @@
> +  let smartReplyButton    = document.getElementById("hdrSmartReplyButton");
> +  let replyOnlyButton     = document.getElementById("hdrReplyOnlyButton");
> +  let replyButton         = document.getElementById("hdrReplyButton");
> +  let replyAllButton      = document.getElementById("hdrReplyAllButton");
> +  let replyListButton     = document.getElementById("hdrReplyListButton");

Please don't line these up.  It makes it harder to add stuff later.  (There's also another example or two further down that I'ld like you to change…)

::: mail/base/content/msgHdrViewOverlay.xul
@@ -212,0 +212,5 @@
> > +                <toolbarbutton id="hdrReplyOnlyButton" label="&hdrReplyButton.label;"
> > +                               tooltiptext="&hdrReplyButton.tooltip;"
> > +                               oncommand="MsgReplyMessage(event);RestoreFocusAfterHdrButton();"
> > +                               observes="button_reply"
> > +                               class="toolbarbutton-1 msgHeaderView-button hdrReplyToSenderButton"/>

Should this toolbarbutton have the hdrReplyToSenderButton class, or should we add a new class for it, or should it use the hdrReplyButton class?

Comment 4

[Jim Porter (:squib)]

(In reply to comment #3)
> Should this toolbarbutton have the hdrReplyToSenderButton class, or should
> we add a new class for it, or should it use the hdrReplyButton class?

Hmm. In general, it should always look like the reply to sender button, but maybe a theme would want to make one of them different. I'm not sure that the hdrReplyButton class is right, since that's a dual-button, and this is just a regular button. I guess the most flexible solution is to give it a new class.

One other thing I noticed that's kind of confusing: when you have the "reply to sender" button and the "smart reply" button on the toolbar, and you customize it while looking at a message sent only to you (i.e. the "smart reply" button is showing "reply only"), you see two seemingly identical reply buttons, so you don't know which one is the smart reply and which one is the reply to sender.

I'm not totally sure how to fix that, but here are a couple of possibilities:

 1) give the "smart reply" toolbaritem a label so at least you know what it is
    in the Customize dialog
 2) when customizing and the smart reply button is on the toolbar, change its
    label to be "smart reply" (maybe by way of a dummy button)
 3) when customizing and the smart reply button is on the toolbar, set it to the
    "reply all" mode to disambiguate it

Any preferences?

Comment 5

[Jim Porter (:squib)]

Attachment: Make customization work better

I went with (1) and (2) from comment 4 with this patch. I also cleaned up the UpdateReplyButtons function a bit more, so the logic there should be clearer.

Forewarning: I'm not sure if labels on toolbaritems show up in the Customize dialog on Mac, so the smart reply button might not have a label there. If that's still an issue, it's probably somewhere in toolkit, since I know it works in Linux (and I'm pretty sure it works in Windows too).

Comment 6

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 545060 [details] [diff] [review]
Make customization work better

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

Okay, I've checked this over, and it seems okay, so I guess that means r=me!

Thanks,
Blake.

Comment 7

[Jim Porter (:squib)]

Checked in: http://hg.mozilla.org/comm-central/rev/b9ce47b3670b