Add "Reply to Sender" button to message header for NNTP posts

RESOLVED FIXED in Thunderbird 14.0

Status

Thunderbird
Message Reader UI
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: squib, Assigned: squib)

Tracking

(Depends on: 1 bug)

unspecified
Thunderbird 14.0
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

6 years ago
Created attachment 588781 [details] [diff] [review]
Split "reply" and "followup"

Currently, the reply button in the message header "replies" to the newsgroup. It also lets you "reply all" to the newsgroup and the poster, but there's no button to reply just to the poster. There's a menu item to do this, but no button. It's especially strange when you swap between a mailing list and a newsgroup, which are conceptually very similar, but have a different button layout.

This has been bugging me for a while, so I fixed it locally. Here's the patch for that. Strings might need some work, though.
Attachment #588781 - Flags: ui-review?(bwinton)
Attachment #588781 - Flags: review?(bwinton)
(Assignee)

Comment 1

6 years ago
Created attachment 588782 [details]
What it looks like

And this is what it looks like.
Assignee: nobody → squibblyflabbetydoo
Status: NEW → ASSIGNED
(Assignee)

Updated

6 years ago
Attachment #588782 - Attachment is patch: false
Attachment #588782 - Attachment mime type: text/plain → image/png
Comment on attachment 588781 [details] [diff] [review]
Split "reply" and "followup"

It seems to work pretty well.  ui-r=me.

One thing I noticed is that the icon when the Smart Reply is in the customize dialog varies.  We should probably fix that in a followup bug.

So, I'm guessing you changed "reply" to "replyNews", and "replyOnly" to "reply"?

>+++ b/mail/base/content/mailWindowOverlay.js
>@@ -1028,7 +1028,7 @@
>   if (gFolderDisplay.selectedMessageIsNews)
>   {
>     // News messages always default to the "reply" dual-button.

I think this comment isn't correct anymore.

>-    buttonToShow = "reply";
>+    buttonToShow = "replyNews";

(Also, is "replyNews" actually "followup", and if so, could we rename it to that?)

>+++ b/mail/base/content/msgHdrViewOverlay.xul
>@@ -297,7 +277,33 @@
>+                <toolbarbutton id="hdrReplyNewsButton" label="&hdrReplyNewsButton1.label;"
>+                               tooltiptext="&hdrReplyNewsButton1.tooltip;"
>+                               oncommand="MsgReplyGroup(event);RestoreFocusAfterHdrButton();"
>+                               observes="button_reply"

That observes value seems wrong.  (I don't think there is a button_replynews, but I do think we want to create one for this.)

>+                               class="toolbarbutton-1 msgHeaderView-button hdrReplyNewsButton"
>+                               type="menu-button">
>+                  <menupopup id="hdrReplyNewsDropdown">
>+                    <menuitem id="hdrReplyNews_ReplySubButton"
>+                              label="&hdrReplyNewsButton1.label;"
>+                              tooltiptext="&hdrReplyNewsButton1.tooltip;"
>+                              observes="button_reply"/>

That observes value seems wrong.  (There may not be a 

>+                    <menuseparator id="hdrReplyNewsSubSeparator"/>
>+                    <menuitem id="hdrReplyNews_ReplyAllSubButton"
>+                              label="&hdrReplyAllButton1.label;"
>+                              tooltiptext="&hdrReplyAllButton1.tooltip;"
>+                              observes="button_replyall"
>+                              oncommand="MsgReplyToAllMessage(event); event.stopPropagation();
>+                                         RestoreFocusAfterHdrButton();"/>
>+                    <menuitem id="hdrReplyNews_ReplySubButton"
>+                              label="&hdrReplyButton1.label;"
>+                              tooltiptext="&hdrReplyButton1.tooltip;"
>+                              observes="button_replyall"

I don't think this one is correct, either.  ;)

>+++ b/mail/themes/qute/mail/primaryToolbar-aero.css
>@@ -254,7 +254,7 @@
>     #button-previous, #button-mark, #button-tag, #button-goback,
>     #button-goforward, #button-compact, #button-archive, #hdrArchiveButton,
>     #hdrReplyButton, #hdrReplyToSenderButton, #hdrReplyAllButton,
>-    #hdrReplyOnlyButton, #hdrReplyListButton, #hdrForwardButton,
>+    #hdrReplyListButton, #hdrReplyNewsButton, #hdrForwardButton,
>     #hdrTrashButton, #hdrJunkButton
>     ) .toolbarbutton-icon {
>   margin: 0 !important;

Why is this change only made for aero?  (I'm guessing it's because this rule only exists in aero, but I want to double-check that.)

And I'm going to say r-, so that I can take a look at the button_replynews code.

Thanks,
Blake.
Attachment #588781 - Flags: ui-review?(bwinton)
Attachment #588781 - Flags: ui-review+
Attachment #588781 - Flags: review?(bwinton)
Attachment #588781 - Flags: review-
(Assignee)

Comment 3

6 years ago
(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #2)
> One thing I noticed is that the icon when the Smart Reply is in the
> customize dialog varies.  We should probably fix that in a followup bug.

Filed as bug 718860, since it was about 5 minutes of actual work.

> So, I'm guessing you changed "reply" to "replyNews", and "replyOnly" to
> "reply"?

Right.

> (Also, is "replyNews" actually "followup", and if so, could we rename it to
> that?)

Do you want that change everywhere? I was calling it "replyNews" to keep it in line with the other smart reply buttons, but I'm not strongly opposed to calling it "followup" in the code too.

> Why is this change only made for aero?  (I'm guessing it's because this rule
> only exists in aero, but I want to double-check that.)

Yeah, it's an aero-only thing. It resizes the icons for the buttons, which are 18x18 on aero. It's not the best way of doing things, but we can file another bug to simplify that CSS if need be.
(In reply to Jim Porter (:squib) from comment #3)
> (In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #2)
> > One thing I noticed is that the icon when the Smart Reply is in the
> > customize dialog varies.  We should probably fix that in a followup bug.
> Filed as bug 718860, since it was about 5 minutes of actual work.

Cool.

> > So, I'm guessing you changed "reply" to "replyNews", and "replyOnly" to
> > "reply"?
> Right.

Cool.

> > (Also, is "replyNews" actually "followup", and if so, could we rename it to
> > that?)
> Do you want that change everywhere? I was calling it "replyNews" to keep it
> in line with the other smart reply buttons, but I'm not strongly opposed to
> calling it "followup" in the code too.

I think so, since it's only kinda a reply button.  (The other all have "Reply" in their name, iirc.)

> > Why is this change only made for aero?  (I'm guessing it's because this rule
> > only exists in aero, but I want to double-check that.)
> Yeah, it's an aero-only thing. It resizes the icons for the buttons, which
> are 18x18 on aero. It's not the best way of doing things, but we can file
> another bug to simplify that CSS if need be.

Cool.

Thanks,
Blake.
(Assignee)

Comment 5

6 years ago
I think I'm going to put this on hold for a bit to focus on cloud file stuff. So I don't forget, here are some of the things that we should probably handle here or in another bug:

* Hide "Reply to List" in newsgroups
* Rename "Reply to Newsgroup" in the menus
* Fix bug 498448
(Assignee)

Comment 6

5 years ago
Created attachment 607002 [details] [diff] [review]
Address review comments

This should resolve all the issues with the previous review. I also updated some strings, notably that "Reply to Newsgroup" in the Message and context menus now reads "Followup to Newsgroup". Consistency is good. :)
Attachment #588781 - Attachment is obsolete: true
Attachment #607002 - Flags: ui-review?(bwinton)
Attachment #607002 - Flags: review?(bwinton)
Comment on attachment 607002 [details] [diff] [review]
Address review comments

I like it, both the ui and the code.

r=me, ui-r=me!

Thanks,
Blake.
Attachment #607002 - Flags: ui-review?(bwinton)
Attachment #607002 - Flags: ui-review+
Attachment #607002 - Flags: review?(bwinton)
Attachment #607002 - Flags: review+
(Assignee)

Comment 8

5 years ago
Checked in: http://hg.mozilla.org/comm-central/rev/f5d3b855f290
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 14.0
(Assignee)

Updated

5 years ago
Depends on: 747298
Depends on: 788304
Depends on: 789400
You need to log in before you can comment on or make changes to this bug.