Column Size has tooltip "Click to sort by size" instead of "Sort by size" in normal message list and in message search results list

RESOLVED FIXED in Thunderbird 41.0

Status

Thunderbird
Folder and Message Lists
--
minor
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: aryx, Assigned: Thomas D. (currently busy elsewhere; needinfo?me))

Tracking

Trunk
Thunderbird 41.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Latest Earlybird and Daily on Windows 8.1 Pro 64 bit

When searching for messages using menu Edit > Find > Search Messages and after adding the Size column, moving the mouse over the column header shows the tooltip "Click to sort by size" (sizeColumnTooltip from messenger.properties) instead of "Sort by size" (sizeColumn2.tooltip introduced by bug 881073).

The function which does this is SetNewsFolderColumns ( http://mxr.mozilla.org/comm-aurora/source/mail/base/content/commandglue.js#69 ) and this happens despite its name also when a mail inbox is selected.
Summary: Message search: Column Size has tooltip "Click to sort by size" instead of "Sort by size" → Column Size has tooltip "Click to sort by size" instead of "Sort by size" in normal message list and in message search results list

Comment 1

3 years ago
Zach, as you fixed bug 881073, would you like to finish this one too?
Flags: needinfo?(zach.x.nickell)

Comment 2

3 years ago
(In reply to :aceman from comment #1)
> Zach, as you fixed bug 881073, would you like to finish this one too?

Hi Aceman, I'd be happy to work on this bug:)
Flags: needinfo?(zach.x.nickell)

Comment 3

3 years ago
So you need to update the strings sizeColumnTooltip and linesColumnTooltip in the files
/mail/locales/en-US/chrome/messenger/messenger.properties
/suite/locales/en-US/chrome/mailnews/messenger.properties

Comment 4

3 years ago
Created attachment 8391914 [details] [diff] [review]
Corrects inconsistency in sizeColumnTooltip and linesColumnTooltip
Attachment #8391914 - Flags: review?(bwinton)
Comment on attachment 8391914 [details] [diff] [review]
Corrects inconsistency in sizeColumnTooltip and linesColumnTooltip

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

So, I like where this has started from, but…
1) I think this is a material change in the strings, and so we'll need to give them new IDs so that the localizers notice.
2) I'm not a reviewer for the change in /suite/, so you'll need to get someone to review those parts.  (Standard8 or Neil have reviewed other changes to that file.)

Thanks,
Blake.
Attachment #8391914 - Flags: review?(bwinton) → review-

Comment 6

3 years ago
Created attachment 8391936 [details] [diff] [review]
Corrects inconsistency in sizeColumnTooltip and linesColumnTooltip (with new localization keys )

I figured I'd have to change the localization keys. Sorry about that, I should have asked first
Attachment #8391914 - Attachment is obsolete: true
Attachment #8391936 - Flags: review?(standard8)
Attachment #8391936 - Flags: review?(bwinton)
Comment on attachment 8391936 [details] [diff] [review]
Corrects inconsistency in sizeColumnTooltip and linesColumnTooltip (with new localization keys )

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

Yep, seems good to me.  Thanks!
Attachment #8391936 - Flags: review?(bwinton) → review+

Comment 8

3 years ago
nit: prefer adding 2 instead of 1 as 1 an l are very similar in many fonts.
Attachment #8391936 - Flags: review?(standard8) → review?(neil)

Comment 9

3 years ago
Comment on attachment 8391936 [details] [diff] [review]
Corrects inconsistency in sizeColumnTooltip and linesColumnTooltip (with new localization keys )

Yes, you should use 2 for consistency with sizeColumn2.tooltip etc.
Attachment #8391936 - Flags: review?(neil) → review+

Comment 10

3 years ago
Created attachment 8392388 [details] [diff] [review]
Corrects inconsistency in sizeColumnTooltip and linesColumnTooltip (with new localization keys) v1.1

Updated

3 years ago
Attachment #8392388 - Flags: feedback?(mkmelin+mozilla)

Comment 11

3 years ago
Comment on attachment 8392388 [details] [diff] [review]
Corrects inconsistency in sizeColumnTooltip and linesColumnTooltip (with new localization keys) v1.1

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

Didn't test it but looks ok to me!
Attachment #8392388 - Flags: feedback?(mkmelin+mozilla) → feedback+
Unfortunately nobody requested checkin-needed on this little patch, so it's already starting to bitrot...

Zach, thanks for the patch. Next time, when you have all the reviews (as you do for this bug), don't forget to set the keyword checkin-needed, otherwise it won't get included into Trunk (the main code branch).

Pls update the patch, at least for the two messenger.properties files the line numbers have changed.

Updated

3 years ago
Attachment #8391936 - Attachment is obsolete: true

Comment 13

3 years ago
Zach, will you fix the patch per comment 12?
Assignee: nobody → zach.x.nickell
Status: NEW → ASSIGNED
Flags: needinfo?(zach.x.nickell)
(In reply to :aceman from comment #13)
> Zach, will you fix the patch per comment 12?

I reached out to Zach but no reply
Status: ASSIGNED → NEW
Flags: needinfo?(zach.x.nickell)
Whiteboard: [patchlove]
Assignee: zach.x.nickell → nobody
Severity: normal → minor
Assignee: nobody → bugzilla2007
OS: Windows 8.1 → All
Hardware: x86_64 → All
Created attachment 8624492 [details] [diff] [review]
unbitrotted patch - polish size and lines column tooltips
Status: NEW → ASSIGNED
Comment on attachment 8624492 [details] [diff] [review]
unbitrotted patch - polish size and lines column tooltips

r=bwinton from comment 7
r=neil from comment 9
ui-r=ThomasD
Attachment #8624492 - Flags: ui-review+
Attachment #8624492 - Flags: review+
Keywords: checkin-needed
Whiteboard: [patchlove]
Attachment #8392388 - Attachment is obsolete: true

Comment 17

2 years ago
https://hg.mozilla.org/comm-central/rev/103550b81b55

Updated

2 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 41.0
You need to log in before you can comment on or make changes to this bug.