Closed
Bug 1190609
Opened 10 years ago
Closed 9 years ago
Remove confusing asterisk from subfolder message count
Categories
(Thunderbird :: Folder and Message Lists, defect)
Thunderbird
Folder and Message Lists
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 45.0
People
(Reporter: aleth, Assigned: aceman)
References
Details
Attachments
(5 files, 3 obsolete files)
It's basically impossible to guess what it means:
https://twitter.com/supersole/status/626106596175646720
And it's redundant with the > arrow to the left.
| Reporter | ||
Comment 1•10 years ago
|
||
Was there another reason this was added in the first place?
Flags: needinfo?(acelists)
Comment 2•10 years ago
|
||
The asterisk was added because it was easy to forget that the number represented a summary of subfolders, and not a value for a particular folder. I've had that confusion in the past myself, seeing a number that seemed to conflict with the actual folder count, and assuming there was a bug somewhere.
It is not redundant with the > arrow (if you mean the folder expanding grippy) as that one is always there. The asterisk (*) is there only when the counts are accumulated from subfolders. That is controlled by a pref (mail.folderpane.sumSubfolders), not the arrow.
I can understand it can be confusing as users do not know what the asterisk means. But nobody proposed a better solution yet. I mean while keeping the feature of accumulating subfolders toggleable (at least by a pref). Also the asterisk is localizable.
Depends on: 464973
Flags: needinfo?(acelists)
Maybe it should simply be fixed so that it doesn't show an asterisk unless the total actually includes unread messages from one or more sub-folders?
As it stands now, even if there are no unread messages in any sub-folders, and one or more unread messages in the parent folder, it still shows the asterisk.
This is what is confusing to me.
If the asterisk only showed up if one or more of the unread messages actually resided in a sub-folder, it would make much more sense.
| Reporter | ||
Comment 5•10 years ago
|
||
(In reply to :aceman from comment #3)
> It is not redundant with the > arrow (if you mean the folder expanding
> grippy) as that one is always there.
Why wouldn't it be more intuitive to
1) include the subfolders in the count when the subfolders are collapsed (> arrow), but
2) not to sum the count when the subfolders are shown (v arrow)?
Is the idea to provide a way to *also* include subfolders in the count when the subfolders are shown?
(In reply to Charles from comment #4)
> Maybe it should simply be fixed so that it doesn't show an asterisk unless
> the total actually includes unread messages from one or more sub-folders?
>
> As it stands now, even if there are no unread messages in any sub-folders,
> and one or more unread messages in the parent folder, it still shows the
> asterisk.
>
> This is what is confusing to me.
Ah, I can understand this specific case. Maybe we could fine-tune this.
> If the asterisk only showed up if one or more of the unread messages
> actually resided in a sub-folder, it would make much more sense.
The asterisk is not specifically about unread messages. It summarizes the numbers from subfolders in the respective column, so also folder size, message count and yes, also unread msg count, if you have that enabled.
(In reply to aleth [:aleth] from comment #5)
> Why wouldn't it be more intuitive to
> 1) include the subfolders in the count when the subfolders are collapsed (>
> arrow), but
> 2) not to sum the count when the subfolders are shown (v arrow)?
I think that is the current behaviour. Do you see something else?
Comment 7•10 years ago
|
||
> The asterisk is not specifically about unread messages.
Right. In the absence an unread column count, bold is what communicates the existence of unread messages.
And *color* communicates new unseen messages.
Comment 8•10 years ago
|
||
(In reply to :aceman from comment #3)
> It is not redundant with the > arrow (if you mean the folder expanding
> grippy) as that one is always there. The asterisk (*) is there only when the
> counts are accumulated from subfolders. That is controlled by a pref
> (mail.folderpane.sumSubfolders), not the arrow.
Isn't that a hidden pref? If you don't change it, it's redundant with the arrow. If you do change it, then you're just disabling the feature entirely. There should never be a situation where some folders are summed recursively and others aren't. I'm not sure what the asterisk actually provides here.
The pref is not hidden, it is listed normally in the all-thunderbird.js file.
It is not redundant. The comment in the code itself explains it:
// Only show counts / total size of subtree if the pref is set,
// we are in "All folders" mode and this folder row is not expanded.
So how can these 3 conditions be the same as only the last one (row not expanded = arrow) ?
Comment 10•10 years ago
|
||
(In reply to :aceman from comment #6)
> The asterisk is not specifically about unread messages. It summarizes the
> numbers from subfolders in the respective column, so also folder size,
> message count and yes, also unread msg count, if you have that enabled.
Ah, I keep forgetting about the other coluns because I never ever use them. The only thing that makes sense - to me - to be able to see at a glance is message unread count. If I need more details, I drill down, and check the status bar.
> (In reply to aleth [:aleth] from comment #5)
>> Why wouldn't it be more intuitive to
>> 1) include the subfolders in the count when the subfolders are collapsed (>
>> arrow), but
>> 2) not to sum the count when the subfolders are shown (v arrow)?
>
> I think that is the current behaviour. Do you see something else?
This is indeed what I see (no asterisk when sub-folders are expanded).
Comment 11•10 years ago
|
||
(In reply to :aceman from comment #9)
> It is not redundant. The comment in the code itself explains it:
> // Only show counts / total size of subtree if the pref is set,
> // we are in "All folders" mode and this folder row is not expanded.
>
> So how can these 3 conditions be the same as only the last one (row not
> expanded = arrow) ?
Well, it is redundant in that it displays the asterisk even if there are no messages in any sub-folders that meet the criteria (ie, no unread messages in sub-folders, only in parent folder)...
So, I guess what is needed is a bit of tweaking to the behavior, so the asterisk only displays when there are messages in sub-folders that meet the criteria (based on what columns are enabled)...
| Assignee | ||
Comment 12•10 years ago
|
||
Yes, that looks like a good proposal that I could implement. It looks to me it is the same as bug 1185417? If yes, I could implement it there and leave the bug here for any further ideas aleth may have. Implementing the tweak you propose still does not make the users know more what the asterisk actually means :)
Assignee: nobody → acelists
Comment 13•10 years ago
|
||
Yes, perfect - but dunno what to do to make it more clear to users...
Thanks!
| Reporter | ||
Comment 14•10 years ago
|
||
(In reply to :aceman from comment #9)
> The pref is not hidden, it is listed normally in the all-thunderbird.js file.
>
> It is not redundant. The comment in the code itself explains it:
> // Only show counts / total size of subtree if the pref is set,
> // we are in "All folders" mode and this folder row is not expanded.
>
> So how can these 3 conditions be the same as only the last one (row not
> expanded = arrow) ?
I think what we were trying to get at is that *if* you have the pref set, a non-expanded row in All Folders will *always* come with an asterisk, so it's redundant in that it carries no information (it will show up on all such folders). Of course that will no longer be true if comment 12 happens...
| Reporter | ||
Comment 15•10 years ago
|
||
(In reply to Charles from comment #13)
> Yes, perfect - but dunno what to do to make it more clear to users...
Would replacing "(*5)" with "(5 in subfolders)" be too long?
(Note the number is at the beginning, so if the string gets cut off that's not necessarily terrible.)
Comment 16•10 years ago
|
||
(In reply to aleth [:aleth] from comment #15)
> Would replacing "(*5)" with "(5 in subfolders)" be too long?
For me, yes, absolutely - please don't do that - unless it is an option.
But if you were going to do it that way, I'd do:
(5, 3 in sub-folders) indicating there are 5 total, 2 in the parent folder and 3 in one or more sub-folders).
But that is just plain ugly to me... ;)
Comment 17•10 years ago
|
||
(In reply to aleth [:aleth] from comment #15)
> Would replacing "(*5)" with "(5 in subfolders)" be too long?
> (Note the number is at the beginning, so if the string gets cut off that's
> not necessarily terrible.)
This would IMHO make the folder pane too wide. What about (5+), + for additional folders?
Comment 18•10 years ago
|
||
Or better:
5:2,3
or something like that...
Comment 19•10 years ago
|
||
(In reply to Charles from comment #18)
> Or better:
>
> 5:2,3
>
> or something like that...
IMHO "5:2,3" or maybe just "2,3" could be a good solution, but only for the unread count, and only for users who do not display the unread count in a column, but allow the unread count to trail the folder name. To show two or three separate numbers in a column, for either unread or total or size, would (IMHO) make the columns too wide and would probably be confusing.
| Assignee | ||
Comment 20•10 years ago
|
||
(In reply to aleth [:aleth] from comment #15)
> Would replacing "(*5)" with "(5 in subfolders)" be too long?
> (Note the number is at the beginning, so if the string gets cut off that's
> not necessarily terrible.)
Yes, that would be very ugly. Also the other proposals like 5:2,3 seem non-working as we need to preserve right aligning the numbers in the columns.
Maybe the last proposal of only doing this right after the folder name (no column) would work as the right aligning is not needed there.
| Reporter | ||
Comment 21•10 years ago
|
||
Some ideas given these constraints:
1) How about dropping the asterisk and instead adding a verbose tooltip (for (5), "2 here plus 3 in subfolders" or similar). That way if the user is surprised at the value of the number, they get a detailed explanation should they want one, in a discoverable way.
2) (v5) where v is a downward arrow similar to the one in the tree, ideally in the same colour (grey), that expands the folder entry if clicked. Again, the idea is to make the meaning discoverable. (Possible problem: I'm not sure tree items allow this kind of styling.)
Comment 22•10 years ago
|
||
(In reply to :aceman from comment #20)
> Maybe the last proposal of only doing this right after the folder name (no
> column) would work as the right aligning is not needed there.
Actually, that is what I was talking about, for the Unread count - sorry for not being clear.
I just looked at this more closely, and I don't think this should even be attempted for the column data, only for the Unread count in parenthesis right after the folder name.
Comment 23•10 years ago
|
||
(In reply to aleth [:aleth] from comment #21)
> 1) How about dropping the asterisk and instead adding a verbose tooltip (for
> (5), "2 here plus 3 in subfolders" or similar).
I actually like this idea, but...
I would leave the asterisk once the issue with the logic is fixed (ie, there should be no asterisk if all of the messages the criteria in question are in the parent folder).
So, when the user sees an asterisk, they could mouse-over to getr a much more informative tooltip.
| Reporter | ||
Comment 24•10 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #17)
> This would IMHO make the folder pane too wide. What about (5+), + for
> additional folders?
The problem with 5+ is that it's commonly used for "5 and above", as in "age 5+".
Comment 25•10 years ago
|
||
Some options:
"Folder (5+3)", where 5 is the number of unread messages in Folder, and 3 is the number in subfolders.
"Folder (5)", where the text is underlined if there are unread messages in subfolders (similar to how a collapsed thread is underlined if there are unread replies).
Comment 26•10 years ago
|
||
Comment 27•10 years ago
|
||
Comment 28•10 years ago
|
||
I have attached two screenshots (posteingang1.png and posteingang2.png) that show a problem with the current implementation:
If you have an unread mail in a folder and no unread mails in the subfolder, the asterisk should not be rendered since the count is correct even if the folder view is collapsed.
| Assignee | ||
Comment 29•10 years ago
|
||
(In reply to Sven Neuhaus from comment #28)
> If you have an unread mail in a folder and no unread mails in the subfolder,
> the asterisk should not be rendered since the count is correct even if the
> folder view is collapsed.
If you want this, bug 1185417 is for you.
| Reporter | ||
Comment 30•9 years ago
|
||
Now bug 1185417 is fixed for 45, it would be nice to make the explanation discoverable by modifying the tooltip when the asterisk is present (to include something like "8 unread, of which 2 in subfolders").
Flags: needinfo?(acelists)
| Reporter | ||
Comment 31•9 years ago
|
||
One last idea: In addition to the tooltip, to avoid confusion with multiplication and to be more reminiscent of collapsed subfolders, one could replace the asterisk (*4) with the triangular bullet, (‣4).
| Reporter | ||
Comment 32•9 years ago
|
||
cc'ing Paenglab for UI input on comment 31 (unlike comment 30, I'm not sure if it's a worthwhile improvement or not).
Flags: needinfo?(richard.marti)
Comment 33•9 years ago
|
||
Can we be sure this triangular bullet is in all fonts? If yes, a triangular bullet directing down could be better to point to folders below the main folder.
If we are not sure it's better to stay on the asterisk.
Flags: needinfo?(richard.marti)
| Reporter | ||
Comment 34•9 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #33)
> Can we be sure this triangular bullet is in all fonts? If yes, a triangular
> bullet directing down could be better to point to folders below the main
> folder.
The asterisk only shows up when the subfolders are collapsed, so a triangular arrow pointing down wouldn't point at anything, if that's what you were thinking?
> If we are not sure it's better to stay on the asterisk.
U+2023 ‣ triangular bullet (HTML ‣) is in the "general punctuation" unicode block.
▼ - U+25BC BLACK DOWN-POINTING TRIANGLE is in "Geometric shapes".
▾ - U+25BE SMALL BLACK DOWN-POINTING TRIANGLE as well.
Examples: (▾5) (▼5) (‣5) (*5)
I don't know how you find out how common support for these is.
Comment 35•9 years ago
|
||
(In reply to aleth [:aleth] from comment #34)
> (In reply to Richard Marti (:Paenglab) from comment #33)
> > Can we be sure this triangular bullet is in all fonts? If yes, a triangular
> > bullet directing down could be better to point to folders below the main
> > folder.
>
> The asterisk only shows up when the subfolders are collapsed, so a
> triangular arrow pointing down wouldn't point at anything, if that's what
> you were thinking?
Pointing was wrong wording, I hope "a symbol for subfolders" is better.
> > If we are not sure it's better to stay on the asterisk.
>
> U+2023 ‣ triangular bullet (HTML ‣) is in the "general punctuation"
> unicode block.
>
> ▼ - U+25BC BLACK DOWN-POINTING TRIANGLE is in "Geometric shapes".
> ▾ - U+25BE SMALL BLACK DOWN-POINTING TRIANGLE as well.
>
> Examples: (▾5) (▼5) (‣5) (*5)
(▾5) would be my preferred.
> I don't know how you find out how common support for these is.
That could be the problem when we are using this and then on some systems, like maybe older XP, this isn't shown correctly.
| Reporter | ||
Comment 36•9 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #35)
> That could be the problem when we are using this and then on some systems,
> like maybe older XP, this isn't shown correctly.
Windows XP's Arial indeed doesn't seem to have it, while Windows Vista and newer are OK:
http://www.microsoft.com/typography/fonts/font.aspx?FMID=1120
Comment 37•9 years ago
|
||
I'm pretty sure Gecko is smart enough to fall back to alternate fonts if the current one doesn't have the symbol you need.
Comment 38•9 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #35)
> That could be the problem when we are using this and then on some systems,
> like maybe older XP, this isn't shown correctly.
At this point in time, with the already know limited resources Thunderbird development will have, I think we can simply not worry about supporting ancient/unsupported/end-of-life operating systems, don't you?
| Assignee | ||
Comment 39•9 years ago
|
||
I do care for XP for now as I use/watch several such machines (which can't be upgraded, only replaced).
I copied the ▾ character from comment 35 and in DOM Inspector I put it into the "label" attribute of the "Write" toolbar button. It worked fine, albeit it is a but ugly in that font. Uglier than in the post here.
I notice there is a nice down arrow behind the "Get messages" button, called the drop marker. Is that an icon (image) or a character? Could we use that one in the folder tree?
Flags: needinfo?(acelists)
Comment 40•9 years ago
|
||
(In reply to :aceman from comment #39)
> I notice there is a nice down arrow behind the "Get messages" button, called
> the drop marker. Is that an icon (image) or a character? Could we use that
> one in the folder tree?
It's a image: https://dxr.mozilla.org/comm-central/source/mozilla/toolkit/themes/windows/global/arrow/arrow-dn.gif
| Reporter | ||
Comment 41•9 years ago
|
||
(In reply to Charles from comment #38)
> > like maybe older XP, this isn't shown correctly.
>
> At this point in time, with the already know limited resources Thunderbird
> development will have, I think we can simply not worry about supporting
> ancient/unsupported/end-of-life operating systems, don't you?
As far as I know, Firefox ESR 45 will still support XP, and so TB 45 will do the same.
Comment 42•9 years ago
|
||
Instead of the different characters that are being discussed, how
about using an underline and/or a different color to indicate that a
tooltip is available to provide more information? This would follow
a well-established convention. It would have the additional small
advantage of saving a little horizontal space (which is more important
for folder pane columns than for the unread count in parentheses after
the folder name).
| Reporter | ||
Comment 43•9 years ago
|
||
As the improved tooltip will require strings, this should ideally be done soon if it is to be in 45.
| Assignee | ||
Comment 44•9 years ago
|
||
Would this generic message be enough? Or is it really important to show the real numbers for the specific cell inside the tooltip message?
The tooltip already shown only if you hover a cell containing the summarizing arrow. That is in contrast to the other tooltip contents that show on the whole row. I fixed that for the ellipsed folder name to only show if the name itself is hovered. I'm not sure if this should also be done for the new messages previews (whether they should only be shown if the folder name is hovered).
I tried the arrow symbol on Linux (KDE3) and Win XP and it works. Somebody should please try Win 7 and OS X.
Attachment #8697776 -
Flags: ui-review?(richard.marti)
Attachment #8697776 -
Flags: feedback?(aleth)
| Reporter | ||
Comment 45•9 years ago
|
||
Comment on attachment 8697776 [details] [diff] [review]
patch
Review of attachment 8697776 [details] [diff] [review]:
-----------------------------------------------------------------
::: mail/base/content/folderPane.js
@@ +2223,5 @@
> let unread = this._folder.getNumUnread(false);
> let totalUnread = gFolderStatsHelpers.sumSubfolders ?
> this._folder.getNumUnread(true) : unread;
> + subfoldersContributed = (unread != totalUnread);
> + this._summarized.set(aColName, subfoldersContributed);
So in order to include the numbers in the tooltip, you'd have to do something like this instead?
this._count.set(aColName, [unread, totalUnread])
Is that a big complication?
::: mail/base/content/mailWidgets.xml
@@ +2419,5 @@
> gFolderTreeView._tree.getCellAt(event.clientX, event.clientY, row, col, {});
> + if (col.value.id == "folderNameCol") {
> + let cropped = gFolderTreeView._tree.isCellCropped(row.value, col.value);
> + if (tooltipnode.addLocationInfo(msgFolder, cropped))
> + return true;
You probably don't want to return yet - it's possible you want to call addSummarized as well?
::: mail/locales/en-US/chrome/messenger/messenger.dtd
@@ +600,5 @@
> <!ENTITY folderNameColumn.label "Name">
> <!ENTITY folderUnreadColumn.label "Unread">
> <!ENTITY folderTotalColumn.label "Total">
> <!ENTITY folderSizeColumn.label "Size">
> +<!ENTITY subfoldersExplanation.label "Some of the items counted in this total number are located in subfolders of this folder">
This is a very long string for a tooltip. The advantage of including the numbers would also be that it makes it much shorter:
"11 unread, of which 6 in subfolders" or even
"11 unread messages, of which 6 are in subfolders"
| Reporter | ||
Comment 46•9 years ago
|
||
| Assignee | ||
Comment 47•9 years ago
|
||
(In reply to aleth [:aleth] from comment #45)
> So in order to include the numbers in the tooltip, you'd have to do
> something like this instead?
>
> this._count.set(aColName, [unread, totalUnread])
>
> Is that a big complication?
And then get the values for the tooltip. Surely it could be done, I just feel the feature already has quite some code attached, that must be run very often (when redrawing cells in folder pane). So I just ask if we want it :)
> ::: mail/base/content/mailWidgets.xml
> @@ +2419,5 @@
> > gFolderTreeView._tree.getCellAt(event.clientX, event.clientY, row, col, {});
> > + if (col.value.id == "folderNameCol") {
> > + let cropped = gFolderTreeView._tree.isCellCropped(row.value, col.value);
> > + if (tooltipnode.addLocationInfo(msgFolder, cropped))
> > + return true;
>
> You probably don't want to return yet - it's possible you want to call
> addSummarized as well?
The current tooltip logic is that the first highest priority text wins. There is currently a text for new messages, then for full folder name (if it is shown ellipsed (cropped)), then for the summarizing prefix.
Do we want to change that logic now and accumulate tooltip texts? If we change it that for the counts the first 2 texts are not shown, problem is solved there. But for the name column, there you can have all the 3 texts (when the unread count is attached to folder name).
> > +<!ENTITY subfoldersExplanation.label "Some of the items counted in this total number are located in subfolders of this folder">
>
> This is a very long string for a tooltip. The advantage of including the
> numbers would also be that it makes it much shorter:
> "11 unread, of which 6 in subfolders" or even
> "11 unread messages, of which 6 are in subfolders"
Yeah :)
| Reporter | ||
Comment 48•9 years ago
|
||
These all seem like UI-r decisions to me ;)
(In reply to :aceman from comment #47)
> (In reply to aleth [:aleth] from comment #45)
> And then get the values for the tooltip. Surely it could be done, I just
> feel the feature already has quite some code attached, that must be run very
> often (when redrawing cells in folder pane). So I just ask if we want it :)
Performance-wise, you're already calculating these numbers, I don't think storing them makes much difference. (I haven't looked at whether the code could be more efficient about how/when it recalculates. How many folders do you think there are, typically? Is it worth worrying about?)
Comment 49•9 years ago
|
||
How it looks on Win10.
I think too the actual tooltiptext is too long. Aleth's proposal would be a lot better, if doable.
| Reporter | ||
Comment 50•9 years ago
|
||
(In reply to :aceman from comment #47)
> The current tooltip logic is that the first highest priority text wins.
> There is currently a text for new messages, then for full folder name (if it
> is shown ellipsed (cropped)), then for the summarizing prefix.
>
> Do we want to change that logic now and accumulate tooltip texts? If we
> change it that for the counts the first 2 texts are not shown, problem is
> solved there. But for the name column, there you can have all the 3 texts
> (when the unread count is attached to folder name).
I don't know this well enough to have an opinion about it.
When the unread count is included in the folder name, some combination like "Foldername (11 unread, of which 6 in subfolders)" might work as well, depending on where the unread count is attached to the name I suppose.
| Assignee | ||
Comment 51•9 years ago
|
||
(In reply to aleth [:aleth] from comment #50)
> (In reply to :aceman from comment #47)
> > The current tooltip logic is that the first highest priority text wins.
> > There is currently a text for new messages, then for full folder name (if it
> > is shown ellipsed (cropped)), then for the summarizing prefix.
> >
> > Do we want to change that logic now and accumulate tooltip texts? If we
> > change it that for the counts the first 2 texts are not shown, problem is
> > solved there. But for the name column, there you can have all the 3 texts
> > (when the unread count is attached to folder name).
>
> I don't know this well enough to have an opinion about it.
>
> When the unread count is included in the folder name, some combination like
> "Foldername (11 unread, of which 6 in subfolders)" might work as well,
> depending on where the unread count is attached to the name I suppose.
This could become complicated with 3 texts to concatenate. I suggest we take it to a new bug.
| Assignee | ||
Comment 52•9 years ago
|
||
Better?
Attachment #8697776 -
Attachment is obsolete: true
Attachment #8697776 -
Flags: ui-review?(richard.marti)
Attachment #8697776 -
Flags: feedback?(aleth)
Attachment #8697789 -
Flags: ui-review?(richard.marti)
Attachment #8697789 -
Flags: feedback?(aleth)
Status: NEW → ASSIGNED
OS: Unspecified → All
Hardware: Unspecified → All
Comment 53•9 years ago
|
||
Comment on attachment 8697789 [details] [diff] [review]
patch v2
Yes, better.
I propose "2 unread messages in this folder, 4 in subfolders". Then it's clearer.
Attachment #8697789 -
Flags: ui-review?(richard.marti) → ui-review+
| Reporter | ||
Comment 54•9 years ago
|
||
Comment on attachment 8697789 [details] [diff] [review]
patch v2
Review of attachment 8697789 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks for looking at this for 45!
I don't know the surrounding code or the various circumstances in which this tooltip is filled well enough to comment further ;)
Attachment #8697789 -
Flags: feedback?(aleth)
| Assignee | ||
Comment 55•9 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #53)
> I propose "2 unread messages in this folder, 4 in subfolders". Then it's
> clearer.
If you want the "unread" word in the string, then we would need to have 3 different strings (as each column has different contents). I intentionally did the string universal to avoid that.
Comment 56•9 years ago
|
||
(In reply to :aceman from comment #55)
> If you want the "unread" word in the string, then we would need to have 3
> different strings (as each column has different contents). I intentionally
> did the string universal to avoid that.
When it's too complicated, then it's okay like it is now.
| Reporter | ||
Comment 57•9 years ago
|
||
Comment on attachment 8697789 [details] [diff] [review]
patch v2
Review of attachment 8697789 [details] [diff] [review]:
-----------------------------------------------------------------
::: mail/locales/en-US/chrome/messenger/messenger.properties
@@ +514,5 @@
> ## This string shows an indication that the value shown is actually a summary
> ## accumulated from all subfolders.
> ## %S = summarized value from all subfolders
> +folderSummarizedSymbolValue=▾%S
> +subfoldersExplanation=%1$S in this folder, %2$S in subfolders
Maybe add a l10n note saying that this string is for the tooltip, and list the kind of items it is used for, so the translators can also translate it "universally".
| Assignee | ||
Comment 58•9 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #56)
> (In reply to :aceman from comment #55)
> > If you want the "unread" word in the string, then we would need to have 3
> > different strings (as each column has different contents). I intentionally
> > did the string universal to avoid that.
>
> When it's too complicated, then it's okay like it is now.
It's not that it would be too complicated, but I think it is overkill :)
| Assignee | ||
Comment 59•9 years ago
|
||
Attachment #8697789 -
Attachment is obsolete: true
Attachment #8697810 -
Flags: review?(mkmelin+mozilla)
Comment 60•9 years ago
|
||
I have tested in TB45a1 and it is fixed.
| Assignee | ||
Comment 61•9 years ago
|
||
Thanks for testing the try build Marco. For others, the patch is not in a proper nightly yet.
Comment 62•9 years ago
|
||
Comment on attachment 8697810 [details] [diff] [review]
patch v2.1
Review of attachment 8697810 [details] [diff] [review]:
-----------------------------------------------------------------
Seems to work fine. r=mkmelin with the changes below
::: mail/base/content/folderPane.js
@@ +2181,5 @@
> this._folderFilter = aFolderFilter;
> this._favicon = null;
> + // Contains message counts for each folder column.
> + // Values are of the format "[value_for_folder, total_value_with_subfolders]".
> + this._summarizedCounts = new Map();
A Map looks like an odd choice for this as the values have no real connection. Just use a plain object, or even a pair.
@@ +2884,5 @@
> + *
> + * @param aSize The size in bytes to format.
> + * @param aUnit Optional unit to use for the format.
> + * Possible values are "KB" or "MB".
> + */
document @return
Attachment #8697810 -
Flags: review?(mkmelin+mozilla) → review+
| Reporter | ||
Comment 63•9 years ago
|
||
Comment on attachment 8697810 [details] [diff] [review]
patch v2.1
Review of attachment 8697810 [details] [diff] [review]:
-----------------------------------------------------------------
::: mail/base/content/folderPane.js
@@ +2181,5 @@
> this._folderFilter = aFolderFilter;
> this._favicon = null;
> + // Contains message counts for each folder column.
> + // Values are of the format "[value_for_folder, total_value_with_subfolders]".
> + this._summarizedCounts = new Map();
A Map isn't a bad choice when you have no control over the values of the keys. No need to worry about hasOwnProperty or folder names like "__proto__" ;)
Comment 64•9 years ago
|
||
There's no name to worry about, it's just two basically unrelated numbers.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map: "If in contrast you have a fixed amount of keys, operate on them individually, and distinguish between their usage, then you want an object."
| Assignee | ||
Comment 65•9 years ago
|
||
Well, there was a version of the patch that iterated over all the keys to get the final result :)
Also, we may add new columns or an extension now could, so there is not a fixed amount of keys.
The keys are column names, so none of them will hopefully be called "__proto__" :)
Comment 66•9 years ago
|
||
Oh right, I misread that. What you should do though is to add a note that the keys are column name
| Assignee | ||
Comment 67•9 years ago
|
||
OK, done, thanks.
Attachment #8697810 -
Attachment is obsolete: true
Attachment #8697885 -
Flags: review+
Keywords: checkin-needed
| Assignee | ||
Comment 68•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 45.0
You need to log in
before you can comment on or make changes to this bug.
Description
•