Closed Bug 614566 Opened 14 years ago Closed 12 years ago

V2.1B1+ Email Size display harder to read

Categories

(MailNews Core :: Backend, defect)

Product:
MailNews Core
Component:
Backend
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: rob, Unassigned)

References

Details

(Whiteboard: [add-on: comment #24][wontfix?]) 

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0b7pre) Gecko/20101008 Firefox/4.0b7pre SeaMonkey/2.1b1
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b7pre) Gecko/20101008 Firefox/4.0b7pre SeaMonkey/2.1b1

In seamonkey 2.0.x the file size display used to show in right-justified kb no matter what size, so the decimals would line up, and it was easy to spot large emails (I used underlines to line up the figures in the list below to simulate this arrangement)
_771 kb
__44 kb
3100 kb
4700 kb
__26 kb


In 2.1B1 and 2.1B2pre the size display goes like this
771 kb
44.4 kb
3.1 mb
4.7 mb
26.4 kb

How do you spot the large emails in that list????

Reproducible: Always

Steps to Reproduce:
1.  Just turn on size display in the email header pane
2.
3.
Actual Results:  
Decimals aren't aligned

Expected Results:  
Decimals should be aligned, so large files can be spotted.

http://forums.mozillazine.org/viewtopic.php?f=6&t=2039897
This was introduced in bug 516787, and since Neil signed off on it (sr+), you may have a hard time to reverse that change. I see your point though that it's more difficult now to identify large messages just by the length of the size string. Maybe adding a preference setting might be a viable option to give the user the choice of one vs. the other.
OS: Windows XP → All
Hardware: x86 → All
Version: unspecified → Trunk
I agree with Rob, this change is only going to make things harder for users. If people really need to know number of megabytes why not use a thousand separator (like a space or a locale specific format)? Then we would have:

__771 kb
___44 kb
3 100 kb
4 700 kb
___26 kb

Either this or just leave things as they were.
That would be quite readable.  To save space, I think the text "kb" could be eliminated.  


__771
___44
3 100
4 700
___26


Important is that emails with large attachments stand out, so they can be cleaned up.  

If we're stuck with the format - as another idea - could at least the mb entries be displayed in BOLD, 

771 kb
44.4 kb
<bold>3.1 mb
4.7 mb</Bold>
26.4 kb





or leave the kb off those and right justify????


___771
____44
3.1 mb
4.7 mb
____26
This was actually caused by a change to core code. But I've CCd the developer of the patch that made the change in case he has any suggestions.
Component: MailNews: Message Display → Backend
Product: SeaMonkey → MailNews Core
QA Contact: message-display → backend
Yes, the other bug was filed against Thunderbird but ended up patching MailNews Core, thus also affecting SeaMonkey. Dropping the "KB" in messenger.properties would be an easy solution (are we really expecting GB-mails somewhere, given most ISPs limit at around 30MB?) but may make it a bit ambiguous. The suggested bolding could be associated with some selectable threshold, different users may have different ideas how large a message has to get to need special attention.
If I were looking for big messages, I'd sort by the size column instead of trying to skim over a potentially long list of messages, so I don't really see where the issue is. The column should still be right-aligned though. They certainly are in Thunderbird, anyway. If it's not right-aligned in Seamonkey, that's definitely a bug.

There's nothing stopping people from making an addon that does things the old way, but I really don't see much point in making it configurable, since it's just another thing that can go wrong.
Or rather, another thing that we'd need tests for to make sure it doesn't go wrong somewhere down the line.
I suppose that a reasonable compromise would be a "Size (KB)" column, totally separate from the current column. That has more-or-less the same result as a pref, but without the added complexity for each field (and it's a lot more discoverable).
I had a look at the suggestion to apply different styles to the sizes, tying
it to the selected KB/MB/GB label. The only location where those are used is
in nsMsgDBView.cpp though, for generating the string, and then forgotten. Thus, in order to be available for a CSS rule allowing a theme to add bold or color, it would have to be passed up to where the column attributes are set, so that option may not be really straight-forward in its implementation either.

> (comment #8) I suppose that a reasonable compromise would be a "Size (KB)" column

It would add some redundancy in the columns, but that's the case already in other columns as well (e.g., reply/forward next to the Subject for Thunderbird or new-message status are shown in icons, in addition to the "Status" column). So, that may be the most viable option after all.
(In reply to comment #9)
> > (comment #8) I suppose that a reasonable compromise would be a "Size (KB)" column
> 
> It would add some redundancy in the columns, but that's the case already in
> other columns as well (e.g., reply/forward next to the Subject for Thunderbird
> or new-message status are shown in icons, in addition to the "Status" column).
> So, that may be the most viable option after all.

The status column reflects in text what some people may not be able to see with icons, so its not quite the same as this situation.

I think adding another column for size is not a good idea. It increases complexity, and I could see it being a little-used/understood option, whichever column style we had as default. Likewise with a preference.

Note that I just checked SeaMonkey on Mac and it uses right-align in both default themes.

I'm cc'ing Bryan as he ui-reviewed the patch and he may have further ideas.
The problematic thing about this bug is that it's too trivial to add a preference or a new column but somehow degrades user experience. I know I can sort by size but nevertheless the obvious visual representation of the size is lost when some sizes are displayed as MB. Unfortunately, such small 'regressions' happen often to Thunderbird or FF while the developers think they are adding a useful feature, and then for example you have to install an add-on just to be able to collapse headers. It would be great if SM didn't try to over-improve the things that work (I understand this has come from TB and would need special work from SM developers).

My opinion is - because a preference for this would indeed could be cluttering SM - that the best solution would be simply to revert to the old behaviour. I don't think anyone would complain about having large sizes displayed as KB, especially that most emails people send are small and don't exceed 100KB.

If that's not possible then maybe sizes with MB could be visually marked somehow for example with a subtle highlight, or prepending an icon or a characted (~ or *) to the size. I don't think that bolding is a good idea because new messages are bold already so this wouldn't work for those.
(In reply to comment #11)
> I don't think anyone would complain about having large sizes displayed as KB,
> especially that most emails people send are small and don't exceed 100KB.

But people did complain. That's what prompted the original bug report.
Yes and no. It sure makes sense to display a size of 8567483 bytes as 8.6 MB while seeing 530 bytes as 0.5 KB is equally satisfactory, and for that reason
I like the change. On the other hand, the visual difference between 8.6 MB and 0.5 KB is fairly small compared to 8567 KB in the old representation.

An important (and increasingly frequent) use case would be a slow/metered line, where you'd likely have offline synchronization switched off and loading MIME parts on demand disabled (bug 529210). In that case, you would like to easily spot large message which you likely don't want to download at this time. Thus, a simple sort by size isn't the solution here if you sort by date to see the most recent arrivals.

This is one of those issues where there are good arguments for doing it either this or that way. Historically, this was enough reason to provide both variants and let the user choose which one to prefer.
(In reply to comment #11)
> If that's not possible then maybe sizes with MB could be visually marked
> somehow for example with a subtle highlight, or prepending an icon or a
> characted (~ or *) to the size. I don't think that bolding is a good idea
> because new messages are bold already so this wouldn't work for those.

This can be done in the current implementation already by overriding the
string definition. In the chrome folder of your installation directory (not
the profile), create a text file named "custom-strings.txt" with

> chrome%3A//messenger/locale/messenger.properties#megaByteAbbreviation2=*** %.*f MB

and the leading "> " removed (I've added it to avoid wrapping of the line). Replace "***" with any indicator you like, e.g., "(!)" or "~~~". Now, any message prompting the "xxx MB" display is marked as "*** xxx MB" and thus easier to spot at first sight in the listing.

While this does the job for me, that's of course just a workaround for advanced users knowing how to apply these hacks, and rather impossible to discover for the regular user without any guidance.
Whiteboard: [workaround: comment #14]
If it's screen space people were worried about, why not have the header and display read (only 6 chars needed for 99 999 kb messages)

SizeKB

__771
___44
3 100
4 700
___26

I'd also be quite happy with a User Choice Column Display
 - either "mixed up" as presently exists in 2.1 for those who would want it (who knows why),
 - or the old format display for those of us who prefer it.

I think the size column is off by default, so users could then pick whatever they want.
(In reply to comment #15)
> If it's screen space people were worried about, why not have the header and
> display read (only 6 chars needed for 99 999 kb messages)

I'm sure people have lots of different reasons for preferring one or the other (e.g. "6.3 MB" may be easier to read for some people, but other people have evolved workflows around reading "6312KB"). I don't think there's a clear "right" answer, although I personally prefer the current way.

For what it's worth, the Windows explorer always shows sizes in KB in the "details" mode, but Nautilus (Gnome's explorer) uses mixed units.

As an aside, I find it interesting that this was reported as a Seamonkey bug and not a Thunderbird bug. If nothing else, it helps to confirm my suspicion that Seamonkey users as a whole have different expectations from Thunderbird/Firefox users. I'm not sure what the right way to go about satisfying both camps would be, though.
(In reply to comment #16)
> For what it's worth, the Windows explorer always shows sizes in KB in the
> "details" mode, but Nautilus (Gnome's explorer) uses mixed units.

It's not consistent on Linux either, "ls -l" gives you 8567483 as the size whereas "ls -lh" shows 8.6M instead. That's what the -h switch is good for.
Please make this configurable in some way, this is the one change that keeps from upgrading from TB 3. (Sorry if my previous attempt to add me to Cc without comment generated emails.)
Is anything happening to this bug? I'm still finding it impossible to upgrade from TB 3.1 just because of this. I would be willing to write an add-on to make the size column configurable. I am a software developer and have worked on a few Firefox and Thunderbird add-ons before, but these were started by others and I'd need some guidance about how to "hook" into the columns display.
Given the recently displayed reluctance of the Thunderbird developers to make any new feature configurable I doubt that there will be any initiative coming from that side. Anyway, the function you are looking for is FormatFileSize() as defined in nsMsgUtils.cpp (currently line 506 in comm-central). Unfortunately this is in the binary part which needs to be compiled, you'd likely have to overlay any function using that utility in the JavaScript part. Thus, it would be easier to approach this in the MailNews Core function directly than intercepting it by an add-on.
(In reply to rsx11m from comment #22)
> Thus, it would be easier to approach this in the MailNews Core function directly than
> intercepting it by an add-on.

It's pretty trivial to add/modify tree columns in an add-on: https://developer.mozilla.org/en/Extensions/Thunderbird/Creating_a_Custom_Column
Whiteboard: [workaround: comment #14] → [add-on: comment #24]
Heh, thanks! I was just digging through DBView myself and got stuck there, certainly quicker if someone takes care of it who knows that part better... :-)
Well, I did have a defunct add-on that did something similar for the Sender/Recipient columns. :) If you're curious about how I did this, I hosted the code on Bitbucket: <https://bitbucket.org/squib/size-in-kb/src>.

In any case, I'm going to recommend we WONTFIX this bug, since an add-on is actually the easiest way to handle this (no need for checking prefs and toggling behavior in C++), and the only way the add-on would break is if the thread pane implementation was rewritten, which would probably result in this feature breaking/going away in core anyway. Doing this as an add-on has the added bonus of making it possible to add the thousands-separator to long numbers (there's no function to do it in C++, unfortunately).

Also, the add-on has been preliminarily reviewed now, so that's good. Here's the message from them:

"It is unclear to us at this time if your add-on will be useful for a general audience so that it warrants public listing. We're granting you preliminary review only, but encourage you to promote your listing so that it garners more active users and user reviews. You may re-nominate your add-on once there is more proof of its usefulness."

So if people want this add-on to be fully-reviewed and publicly listed, use it! If I see a bunch of people using it, I'll renominate it for full review.
Whiteboard: [add-on: comment #24] → [add-on: comment #24][wontfix?]
I've linked to it from Rob's thread in MozillaZine, though looking it up in AMO for SeaMonkey using https://addons.mozilla.org/en-US/seamonkey/addon/size-in-kb/ or Thunderbird for that matter doesn't work at the moment.
I'm very impressed. I was going to start investigating tomorrow, but suddenly (after some testing) I've got exactly I was hoping to achieve. Thanks ever so much!

You need to change line 55 in overlay.js though - size is an undefined variable, hdr.messageSize is what you want.

I'm offering to take that extension "under my wing". (It would be my first one on AMO though, I don't know how to use the site yet and haven't even created an account.) What I'd do (more slowly) is add a preference to deactivate it (restore the default behaviour using messenger.formatFileSize) or maybe offer some more formatting options if anyone requests that, and make the add-on bilingual (English and German).

But first, I will upgrade my Thunderbird. It's so nice that I can do that now. I think the reasoning behind the suggestion to WONTFIX the bug makes sense. Of course I'm not the original reporter.
(In reply to Sebastian Lisken from comment #28)
> You need to change line 55 in overlay.js though - size is an undefined
> variable, hdr.messageSize is what you want.

Fixed and posted to AMO.

> I'm offering to take that extension "under my wing". (It would be my first
> one on AMO though, I don't know how to use the site yet and haven't even
> created an account.) What I'd do (more slowly) is add a preference to
> deactivate it (restore the default behaviour using messenger.formatFileSize)
> or maybe offer some more formatting options if anyone requests that, and
> make the add-on bilingual (English and German).

If it's easier, you can just issue pull requests for the add-on on Bitbucket and I'll manage it. Note that I don't think there should be a preference to deactivate the behavior: you can already do that by disabling the add-on. I'm also not sure about adding other formatting options; I prefer to avoid all-in-one add-ons. Other localizations are certainly welcome, though. You'd only need to search through the l10n tree on MXR to get all the strings (however, I can't find the link to it).
> If it's easier, you can just issue pull requests for the add-on on Bitbucket and
> I'll manage it.

That's okay. I'll have to look into how that works though.

> Note that I don't think there should be a preference to deactivate the
> behavior: you can already do that by disabling the add-on.

I can understand that opinion. Doing it via a preference would basically avoid a restart but there's a logic to saying that the disable function should not be duplicated.

> Other localizations are certainly welcome, though. You'd only need to search
> through the l10n tree on MXR to get all the strings

Why, isn't it just a case of copying the properties file you have now and translating it? With the way things are now, the German file would be identical but of course it would be different in French.

(I'll now be offline for a few hours)
(In reply to Jim Porter from comment #26)
> Also, the add-on has been preliminarily reviewed now, so that's good. Here's
> the message from them:
That's just a general message they give you. Only one of my add-ons got full review, and that was probably because it was restartless.

(In reply to Jim Porter from comment #29)
> Other localizations are certainly welcome, though. You'd
> only need to search through the l10n tree on MXR to get all the strings
If the strings already exist, can you not use the existing strings, rather than including your own locales?
(In reply to Sebastian Lisken from comment #28)
> You need to change line 55 in overlay.js though - size is an undefined
> variable, hdr.messageSize is what you want.

...and I was wondering already where "size" was set globally.  ;-)

(In reply to Jim Porter (:squib) from comment #29)
> Fixed and posted to AMO.

I'm getting a "Not Found" now with either your link or any thunderbird/seamonkey variations on AMO, something seems to have gone wrong.
(In reply to neil@parkwaycc.co.uk from comment #31)
> If the strings already exist, can you not use the existing strings, rather
> than including your own locales?

I tried that, but couldn't figure out how to get Javascript to play nicely with the printf format string ($.*f). It just kept segfaulting.

(In reply to rsx11m from comment #32)
> I'm getting a "Not Found" now with either your link or any
> thunderbird/seamonkey variations on AMO, something seems to have gone wrong.

I tried to be clever and delete the original and upload a fixed version with the same version number, but I apparently totally broke something...
(In reply to Jim Porter from comment #33)
> (In reply to comment #31)
> > If the strings already exist, can you not use the existing strings, rather
> > than including your own locales?
> I tried that, but couldn't figure out how to get Javascript to play nicely
> with the printf format string ($.*f). It just kept segfaulting.
Oh, right, those strings, yeah, you can't really use them from script.
(In reply to neil@parkwaycc.co.uk from comment #34)
> (In reply to Jim Porter from comment #33)
> > I tried that, but couldn't figure out how to get Javascript to play nicely
> > with the printf format string ($.*f). It just kept segfaulting.
> Oh, right, those strings, yeah, you can't really use them from script.

Actually, I suppose I could be clever and just manually replace %.*f with the number I want to use...
And wontfix it is.
Status: UNCONFIRMED → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.