370792 - Improvements to the new attachment UI

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Scott MacGregor]

Spinning this bug off from Bug 223340.

Now that we've landed Wayne's patch for the new attachment UI, we'll probably end up tweaking some styles to get it right.
 
Let's track any of that work here instead of 223340.

Some thoughts I have: we might end up styling windows and Mac OS X differently. For instance on windows we might use small attachment icons + aligning the text to the right of the icon. On Mac we might use large icons with the attachment name below the icon.

Comment 1

[Peter Lairo]

For people with average resolutions (1024x768 @ 17") (nd probably most others too) there needs to be as much space as possible for the filenames. Therefore, the icon on top should be supplanted by (large or small) (I prefer small) icon on the left of the filename.

Also, the "icon on top" has a less organized appearance (i.e., the filenames are not aligned as weel (like a bullet list).

PS. Could someone attach the (three?) relevant screenshots from bug 223340?

Comment 2

[Peter Lairo]

Small icons on left (recognizable icons, maximum space-usage):
https://bugzilla.mozilla.org/attachment.cgi?id=255475

Large icons on left (pretty icons, wasted space):
https://bugzilla.mozilla.org/attachment.cgi?id=255485

Large icons on top (pretty icons, low usability):
https://bugzilla.mozilla.org/attachment.cgi?id=255535

PS. Sorry for the many typos in my previous (and this?) post.

Comment 3

[Wayne Woods]

Minor UI tweak for Windows... what do you think of the following styling to make the attachment splitter look like part of the overall 3pane framework, and match the threadpane splitter in style? Since it's part of the message, maybe there's an argument for it to look disconnected from the 3pane, but I still kinda like this (in userChrome.css format for now):

#messagepanebox {
  border-left: none !important;
}

#attachment-splitter {
  border: none !important;
  min-height: 5px !important;
}

#attachmentView {
  border-left: 1px solid ThreeDShadow !important;
  border-top: 1px solid ThreeDShadow !important;
}

#messagepane {
  border-left: 1px solid ThreeDShadow !important;
}

#msgHeaderViewDeck {
  border-left: 1px solid ThreeDShadow !important;
}

Comment 4

[Jerry Baker]

Can we get it so that attachments displayed inline are not also displayed in the attachment pane? There's no need to see an attachment pane full of image icons when I'm already looking at the images themselves.

I also second the idea that small icons should be used instead of the current large icons. It hogs unnecessary real estate.

Comment 5

[jwq]

Attachment: Claris Emailer attachment list pane

As I wrote in Bug 223340 Comment #134, I'd like to see a list view in the attachments pane. I've attached a screenshot from an old Mac email client, Claris Emailer, which shows the sort of thing that I've found very useful in the past. A list view could include other useful info such as file size, content-type, etc. The file path would be very helpful for detached enclosures.

Comment 6

[Karel Koubek]

(In reply to comment #4)
> Can we get it so that attachments displayed inline are not also displayed in
> the attachment pane? There's no need to see an attachment pane full of image
> icons when I'm already looking at the images themselves.

Please don't do that. For many users the attachment pane is the main clue to see what actually had been attached. I wouldn't like to miss some important attachment somewhere at the end of the message page.

P.S.: The large icons are nice, it doesn't feel so empty when only a few files are attached. What about the second option, 'large icons on left'? It could save some space and be visually more organized.

Comment 7

[Scott MacGregor]

Attachment: screen shot with Wayne's suggested CSS

Comment 8

[Scott MacGregor]

Attachment: UI tweaks

Wayne,

This patch does the following:


1) adds CSS suggestions
2) set the max-height of the attachment box based on the large attachment pref attribute so the height looks right for both large and small attachments.
3) adjust the attachment icon / attachment name orientation via CSS instead of in JS.
4) On Windows and Linux only, try out the attachment name to the right of the icon.

Comment 9

[Peter Lairo]

(In reply to comment #3)
> make the attachment splitter look like part of the overall 3pane framework, and
> match the threadpane splitter in style?

(In reply to comment #7)
> Created an attachment (id=255752) [details]
> screen shot with Wayne's suggested CSS

Won't this confusingly make it a "4-pane" view? I just looked: I does look like a separate pane.

Other bugs/nits(?) (Tb, version 3 alpha 1 (20070218), WinXP):

1. With 1 attachment, when I drag the splitter up, the area below the attachment is not white.

2. When I select an attachment and then click in the *message* area, the attachment's background doesn't return to white.

3. The icons are large, even when there are more that fit the view. :-(

4. The filename is below the the icon (see comment #1 and comment #2) :-(

5. With many attachments and at default height, the vertical scrollbar grippy is useless.

6. With many attachments and at default height, the vertical scroll up & down buttons should scroll one line at a time, and not some random fraction thereof.

7. With many attachments and at default height, there is no indication (other than the scrunched-up scrollbar) that there are more attachments hidden from view. Perhaps (in these cases only?) a "label line" (above the attachments?) might be beneficial: "Attachment/s (21):"

8. Can't right-click in the attachments area and select a different view type (Icons, List, Details). This selection should be remembered across sessions.

Comment 10

[Mike Cowperthwaite]

I basically like the change in style, but for an improved 3-D appearance I added the following to the qute messageHeader.css:
==============================================
#attachmentView
{
  border-left: 1px solid ThreeDShadow;
  border-top: 1px solid ThreeDShadow;
  border-right: 1px solid ThreeDHighLight;     /* add this style... */
  border-bottom: 1px solid ThreeDHighLight;    /* ... and this */
  min-height: 30px;
}
#attachment-splitter {
  border: none;
  border-top: 2px solid;                 /* add this style */
  min-height: 5px;
}
#attachment-splitter[state=collapsed] {  /* add this selector & styles */
  border-bottom: 2px solid;
  min-height: 8px;
}
#msgHeaderViewDeck {
  border-left: 1px solid ThreeDShadow;
  border-right: 1px solid ThreeDHighlight;   /* add this style */
}
==============================================

The 'state=collapsed' selector enhances the appearance of the splitter when it's collapsed to make it more obvious that it's there and available to be dragged.  (I'd like to see the same thing when the Message pane is collapsed -- compare to a collapsed Folder pane.)

The one thing I don't like about that collapsed-splitter styling is, the bottom border is black rather than ThreeDShadow.  This has something to do with the issue described at the previous bug -- you can't set the border color on the splitters when using a 'solid' border.  If you use DOM inspector on a splitter, you'll see that it has these extra styles (for a horizontal splitter):
  -moz-border-top-colors
  -moz-border-bottom-colors
These are each a pair of decimal numbers that I think are intended to give a double-line border for an extra 3D effect, but I don't know what the numbers mean or if there's any way to override them.

Comment 11

[Mike Cowperthwaite]

(In reply to comment #9)
> Won't this confusingly make it a "4-pane" view? I just looked: I does look
> like a separate pane.

I agree, but I'm not sure how to address this.  I still prefer the new appearance: Having one splitter butt up against another while leaving the border in place looks klugey.  

The attachment splitter *is* narrower than the pane splitters, which helps distinguish them.


> 1. With 1 attachment, when I drag the splitter up, the area below the
> attachment is not white.

I don't see this with 2b2/Win2K.  Possibly a Cairo/XP problem?


> 2. When I select an attachment and then click in the *message* area, the
> attachment's background doesn't return to white.

In this case, the background color I see for the attachment is the color for 'selected, not focused' which I'd expect -- or, at least, I'd expect it if the attachment panel were keyboard-accessible, which this isn't.


> 3. The icons are large, even when there are more that fit the view. :-(

So change the setting of |mailnews.attachments.display.largeView|.

I'm finding that every time I install a new version, the preference is getting reset to true (large).  I don't like that!  Especially since setting the pref to small requires a reset.


> 4. The filename is below the the icon (see comment #1 and comment #2) :-(

Not if you change preference.


> 5. With many attachments and at default height, the vertical scrollbar grippy
> is useless.

That's a general problem with scrollbars.

> 6. With many attachments and at default height, the vertical scroll up & down
> buttons should scroll one line at a time, and not some random fraction
> thereof.

Agreed.

> 7. With many attachments and at default height, there is no indication (other
> than the scrunched-up scrollbar) that there are more attachments hidden from
> view. Perhaps (in these cases only?) a "label line" (above the attachments?)
> might be beneficial: "Attachment/s (21):"

I'm not opposed to seeing the label return, but it ends up taking up too much space in every configuration I think of.  Maybe one that appears dynamically with the scrollbars would be OK.


> 8. Can't right-click in the attachments area and select a different view type
> (Icons, List, Details). This selection should be remembered across sessions.

That's an entire new feature, unrelated to this bug.

Comment 12

[Mike Cowperthwaite]

(In reply to comment #10)
> #attachment-splitter {
>   border: none;
>   border-top: 2px solid;                 /* add this style */
>   min-height: 5px;
> }

There is a drawback to this tweak as well -- altho the '2px solid' border on top looks great in the 3-pane, this causes an incorrect border to be drawn in the standalone message window.

Similarly, I notice that this set of style changes from Wayne's original set:
=====================
#messagepanebox {
  border-left: none;
}
#messagepane {
  border-left: 1px solid ThreeDShadow;
}
#msgHeaderViewDeck {
  border-left: 1px solid ThreeDShadow;
}
=====================
only works in the 3-pane; the standalone doesn't have a 3-D shadow border on the left side of the message body -- but then, it doesn't anyway, even in 1.5.  Similarly, the border around the envelope panel (message headers) don't look right in the standalone window either.

Comment 13

[Mike Cowperthwaite]

Attachment: diff file (not really a patch, w.i.p.)

This diff includes my CSS from comment 10, plus what's described below.

I've figured out a bit more regarding the difference between the two window types: messenger.xul (the 3pane) includes mailWindow1.css for styling; messageWindow.xul (the standalone) includes messageWindow.css, which (in qute) has the rule that's introducing most of the broken border problem.  By commenting out the existing rule in messageWindow.css and copying over the #messagepanebox and #messagepane rules from mailWindow1.css, the appearance between the two is much closer -- the left-hand window border and the border between the envelope and the message body now look identical.  (The topmost border between the envelope and the toolbar above it is twice as thick as it should be, but that's due to styling on the toolbar and can be tweaked away.)

But the splitter *still* has the extra |1px solid ThreeDShadow| line drawn above the |1px solid ThreeDHighlight| line.

So the other difference I see is: messageWindow.xul (the standalone) also has a style attribute in the XUL file:  style="height: 0px; min-height: 1px"
So I removed it, and this is where it gets weird: I changed the style in messageWindow.xul -- and now the *3Pane's* message pane shows the extra dark border line at the top of the attachment splitter.

Comment 14

[Mike Cowperthwaite]

Attachment: Screenshot: remaining border issue

(In reply to comment #13)
> So the other difference I see is: messageWindow.xul (the standalone) also has
> a style attribute in the XUL file:  style="height: 0px; min-height: 1px"
> So I removed it, and this is where it gets weird: I changed the style in
> messageWindow.xul -- and now the *3Pane's* message pane shows the extra dark
> border line at the top of the attachment splitter.

This has proved to be unreproducible; I'm running with that same setup now and seeing the bad border only in the standalone window.  Here's a capture from ZoomIn showing the border issue: on the left is the 3-pane, with the border correct (below the "its") and on the right is the standalone with the incorrect border (below the "vers").

Comment 15

[Wayne Woods]

Sorry, had to disappear for a couple of days. Scott, I'll test your patch out tonight and give you feedback (though it looks like it's gathered plenty of feedback already).

With respect to comment 14, this border problem doesn't exist with the styling I suggested in comment 3, does it? Currently in XP I'm seeing the odd border in *both* 3pane and message window.

With respect to comment 9, I can understand the problem of it looking more like a "4pane", which I also mentioned in comment 3. I still like the styling better this way (as others have expressed also), but I don't have my heart set on it. If we go with it, maybe there are other ways of ensuring that the attachments box looks like part of the message.

With respect to comment 4 (= don't show inline attachments in the box as well), I agree with what others have said. I think it's important for them to be shown. If nothing else, the attachments box allows easy manipulation of the attachments as files, which is difficult to do with inline images.

Comment 16

[Scott MacGregor]

It sounds like there's still some border discussion going on. I'm going to remove that part from my patch, leaving just the changes to improve toggling between large and small icons. Will re-attach a fresh patch for you in a few minutes Wayne.

Comment 17

[Mike Cowperthwaite]

(In reply to comment #15)
> With respect to comment 14, this border problem doesn't exist with the
> styling I suggested in comment 3, does it? 

No, it doesn't; but it was lacking the highlight border on top.
I've dug deeper into this and determined that the borders on the splitters aren't designed for adjacency to content areas, but to other 3D-Face areas.  In fact, in the current 3-pane, there is only one border on the splitters -- the left side of the folderpane splitter.  That's in place to force the appearance of a border between the splitter and the folder-view selector header; and it's a bit of a problem because it makes the boundary between rest of the folder pane and the splitter appear slightly off, whether or not there's a scrollbar there.

The better way to fix this is to add the borders I want all around the various content panes.  I've got something close to just how I want it, it needs a bit more tweaking.

Comment 18

[Scott MacGregor]

Attachment: [fixed branch and trunk]make the css support both large and small attachment views

Wayne, this patch makes the css work for both large and small icons.

If using small icons, use different values for the min-height of the attachment box, put the attachment name to the right of the icon.

If using large icons, use a different value for the min-height of the attachment box, put the attachment name below the icon.

In short this patch makes the layout look right for users who toggle the large attachment pref without them having to add their own style rules.

Comment 19

[Mike Cowperthwaite]

Attachment: diff -uw vs 2b2-0221 distro

This styles the qute theme, under Win2K, the way I want:
 * 3D style borders all around the message+envelope, and around the attachment
   panel.
 * Uniform appearance in 3-Pane and Standalone
 * Removes some unnecessary attributes from the XUL files for the 3-Pane and
   Standalone
 * Additionally strips the left-side border of the folder-pane splitter, and 
   adds a similar border on the right edge of the folder-pane header (the folder
   view selector).

This is based on the CSS from Scott's first attachment 255752 [details] here, and includes the additional CSS tweaking from attachment 256120 [details] [diff] [review].

Comment 20

[Mike Cowperthwaite]

Oh, I forgot to note:  in mailWindow1.css, there is this line:
#messagepane {
  border-bottom: 2px solid ThreeDHighlight;
}

The equivalent line for the standalone window only sets a 1px border on the bottom, but for some reason that is insufficient in the 3-pane's.  So this one line is a bit of a hack, but I don't know how to debug the reason it's needed.

Comment 21

[Wayne Woods]

Comment on attachment 256120 [details] [diff] [review]
[fixed branch and trunk]make the css support both large and small attachment views

This looks good in Pinstripe. Tested with small and large icons. I'll get to Mike's follow-up patch next.

Comment 22

[Wayne Woods]

(In reply to comment #19)
> Created an attachment (id=256210) [details]
> diff -uw vs 2b2-0221 distro

I obviously only tested out the stuff in this that might affect Pinstripe, i.e. the .xul files. It didn't seem to affect anything that I could see. (Man, that patch wasn't very friendly to apply.. is there a trick to applying patches in that format?)

Comment 23

[Scott MacGregor]

Hmm, I just do:

patch -p0 < diffile.diff

Comment 24

[Peter Lairo]

(In reply to comment #11)
> (In reply to comment #9)
> > 1. With 1 attachment, when I drag the splitter up, the area below the
> > attachment is not white.
> 
> I don't see this with 2b2/Win2K.  Possibly a Cairo/XP problem?

Possibly. New spin-off bug? Don't you test on trunk?

> > 3. The icons are large, even when there are more that fit the view. :-(
> 
> So change the setting of |mailnews.attachments.display.largeView|.

I pointed this out, not only because it bothers me, but because I think smaller icons on the left will benefit most users (who will never bother to change an internal pref) (see comment #1 and comment #2). (Plus, I'm getting tired of making manual edits)

> > 8. Can't right-click in the attachments area and select a different view type
> > (Icons, List, Details). This selection should be remembered across sessions.
> 
> That's an entire new feature, unrelated to this bug.

It is an "Improvement to the new attachment UI" and therefore clearly "related" to this bug. At worst, it would be a "sub-bug" that this bug depends on (and which could not be marked "fixed" until that bug is fixed). That, or someone needs to rename this bug. ;-)

Anyhow, the improvements so far are great! Let's keep going.

Comment 25

[Magnus Melin [:mkmelin]]

(In reply to comment #22)
> patch wasn't very friendly to apply.. is there a trick to applying patches in
> that format?)

When applying -uw patches you may have to include -l i think, like 
patch -l -p0 < patch.diff

Comment 26

[Karel Koubek]

I think that the attachment pane takes too much space when only a few files are attached (thus occupying only single line) and the message is opened in a separate window.

For example, if I receive a message with three files attached and open it, the attachment pane takes about 1/4 of the window. That's just too much, most of it being only white space.

The space allocated to the attachment pane should be smaller in window view. Even better - scale the pane precisely, according to the number of icon lines (up to the upper hard limit, e.g. 1/4 of the window).

Otherwise I find the new design very nice.

Comment 27

[Wayne Woods]

(In reply to comment #25)
> (In reply to comment #22)
> > patch wasn't very friendly to apply.. is there a trick to applying patches in
> > that format?)
> 
> When applying -uw patches you may have to include -l i think, like 
> patch -l -p0 < patch.diff

Ah, thanks Magnus! I'll try that out next time. I hadn't dealt with -uw patches before, and it kept asking me to locate the files.

Comment 28

[Magnus Melin [:mkmelin]]

Oh, now I see. That patch wasn't created from the cvs source root. (So my advise wouldn't help in this case.) You have to go into the correct dir before applying.

Comment 29

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Wayne, thank you for all the work. The new UI look is great! It's really an improvement.

But there is one thing which I don't like at the moment. If you have a message with one or more attachments which fits into one line no scrollbars are shown. That's like expected. But if you select an attachment the drawn borders increases the size of the attachment block which let us show the scrollbars. :/ That's not neccessary and an user don't expect this behavior.

Comment 30

[Wayne Woods]

Thanks Henrik. I can't reproduce your problem with Thunderbird 3 alpha on Windows XP. I tried both large and small icon stylings, and resizing the message pane, but the borders of the selected icons are still smaller than the minimum attachments box height for me. Could you please tell me what OS you use, what version of Thunderbird, and confirm that you have no third-party themes or extensions installed (or anything in userChrome.css for that matter)? If you are able to, a screenshot might be useful, too. Thanks :)

Comment 31

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Visible scrollbars when selecting an attachment

Wayne, I'm using Thunderbird version 2.0pre (20070304) on Mac OS X. I haven't changed the default preferences, so I think I'm currently using the large icon set. See the attachment where scrollbars are visible when selecting an entry within an one row attachment list.

Comment 32

[Wayne Woods]

Sorry, for some reason I was thinking it was Windows you were on. Yes, I can reproduce this on OS X, too. Not only height changes, but also width, I think. As you select different icons, the others move around slightly. I'll see what I can do quickly to fix this.

Comment 33

[Henrik Skupin [:whimboo][⌚️UTC+1]]

One more minor issue on Mac OS X are the splitter and the min-height like already described in bug 321325 comment 21. If you have more than one row with attachments  you will always see the top pixels (approx. 15-20) of the second row. I'm not able to reduce the height of the attachment panel to only see one row at the same time. If I reduce the height too much the panel collapse and will be hidden.

Comment 34

[Wayne Woods]

(In reply to comment #33)
> If you have more than one row with
> attachments  you will always see the top pixels (approx. 15-20) of the second
> row.

This actually seems like a good thing to me. It's an obvious sign (other than the scrollbar) that the attachments continue.

Comment 35

[Wayne Woods]

Attachment: [fixed branch and trunk]Fixes the moving/jumping attachment icons

Scott, do you think you could please review this before the code freeze? It's a simple fix for the moving icons, which do look bad on Mac (comment 29 to comment 32). It just styles no border for the basic descriptionitem.

Even setting the descriptionitem border to 0 or 1 works. It seems like it's inheriting a negative border from somewhere, causing it to take up less room than it should until it's selected.

Thanks :)

Comment 36

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #34)
> This actually seems like a good thing to me. It's an obvious sign (other than
> the scrollbar) that the attachments continue.

But see the difference between Windows and OS X. On Windows you can only see one row. The next one isn't visible. For me it's an inconsistency.

Comment 37

[Wayne Woods]

Setting the min-height in #attachmentView[largeView="true"] to about 56 or 57px should work, if Scott thinks it's worth it. As long as it doesn't allow a single row of attachments to go into overflow. But I don't have time to test that tonight, and that particular styling seems to be ignored in userChrome.css for some reason.

Comment 38

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Wayne, there is a bug in the pref handling. If you switch the preferences for large or normal attachment view you have to restart Thunderbird to apply the changes. This should be done immediatelly after changing the pref inside the config editor. 

The min-height value for the normal view also seems to be too heigh. I can see one and a half row. Also no scrollbar is visible if more than one row is needed to list the attachments.

Comment 39

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Scott or David, why are all these preferences initialized at the start-up and not updated when necessary while Thunderbird is running and the appropriate preference has changed?

Comment 40

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Sorry, I forgot the link:

http://lxr.mozilla.org/mozilla1.8/source/mail/base/content/msgHdrViewOverlay.js#237

Comment 41

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Ok, these doesn't apply for all global prefs. Some of them are updated by using an observer. I think the largeView pref should also be handled this way.

http://lxr.mozilla.org/mozilla1.8/source/mail/base/content/msgHdrViewOverlay.js#285

Comment 42

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Wayne, one more thing which doesn't work (only tested it on OS X) anymore is starting any action (open, save, deteach and delete) over the context menu without selecting it before. Opening the context of an attachment should focus this attachment and offer the mentioned actions.

Comment 43

[Wayne Woods]

(In reply to comment #38)
> Wayne, there is a bug in the pref handling. If you switch the preferences for
> large or normal attachment view you have to restart Thunderbird to apply the
> changes. This should be done immediatelly after changing the pref inside the
> config editor.

That would be nice, but I don't think it's been a high priority because icon size switching isn't a visible feature anyway. Could you please open a bug for it, and then set it as blocking this one? As I understand it, it's too late to make Thunderbird 2 at this point, but it's worth doing for Tb 3.

> The min-height value for the normal view also seems to be too heigh. I can see
> one and a half row.

I assume "normal view" means small icons. A min-height of 26px would achieve what you want. I'm happy to prepare a patch, but I don't think Scott is accepting anything more for Tb 2 at this stage (unless he says it's OK?).

> Also no scrollbar is visible if more than one row is needed
> to list the attachments.

Are you saying that some attachments are in overflow ("off the screen"), but the scrollbar isn't appearing? I haven't seen this. Can you reproduce it?

(In reply to comment #42)
> Wayne, one more thing which doesn't work (only tested it on OS X) anymore is
> starting any action (open, save, deteach and delete) over the context menu
> without selecting it before.

The context menu appears to be coming from the attachments box and not the descriptionitem icon. I actually wasn't aware that *ever* worked right. At least, it didn't in Tb 1.0 and hasn't in any 2006 build I tested. I can't find a specific bug for it, but it sounds like it's closely related to bug 228107.

Comment 44

[Scott MacGregor]

(In reply to comment #38)
> Wayne, there is a bug in the pref handling. If you switch the preferences for
> large or normal attachment view you have to restart Thunderbird to apply the
> changes. This should be done immediatelly after changing the pref inside the
> config editor. 

People aren't going to toggle this pref back and forth. In fact most users aren't going to even know the pref exists at all let alone change it. We don't need to observe and track this pref live. I'm happy with it being tied to a restart like it currently is. :)

Comment 45

[Scott MacGregor]

Comment on attachment 257348 [details] [diff] [review]
[fixed branch and trunk]Fixes the moving/jumping attachment icons

I'll land this right now!

Comment 46

[Scott MacGregor]

Comment on attachment 257348 [details] [diff] [review]
[fixed branch and trunk]Fixes the moving/jumping attachment icons

fix checked in. Thanks Wayne!

Comment 47

[Scott MacGregor]

(In reply to comment #37)
> Setting the min-height in #attachmentView[largeView="true"] to about 56 or 57px
> should work, if Scott thinks it's worth it. As long as it doesn't allow a
> single row of attachments to go into overflow. But I don't have time to test
> that tonight, and that particular styling seems to be ignored in userChrome.css
> for some reason.

I don't mind checking in a height of 56 or 57 pixels on the Mac tonight, but I don't have access to a Mac right now so I can't verify that the fix will work.

Comment 48

[Peter Lairo]

(In reply to comment #44)
> People aren't going to toggle this pref back and forth.

I actually was planning on doing just that, depending on how many attachments there are and how long the filenames are. I do this regularly in Windows Explorer(R) (when I'm not using Total Commander ;-) ).

> In fact most users
> aren't going to even know the pref exists at all let alone change it. 

Which is why it would be useful to be able to switch from within the context menu (comment #9 item 8). Should I file an RFE for this?

Comment 49

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #43)
> I assume "normal view" means small icons. A min-height of 26px would achieve
> what you want. I'm happy to prepare a patch, but I don't think Scott is
> accepting anything more for Tb 2 at this stage (unless he says it's OK?).

That would be nice at least for Thunderbird 3.

> Are you saying that some attachments are in overflow ("off the screen"), but
> the scrollbar isn't appearing? I haven't seen this. Can you reproduce it?

That seems also Mac related. It was reproducible yesterday. Here on the Windows box it isn't visible.

> The context menu appears to be coming from the attachments box and not the
> descriptionitem icon. I actually wasn't aware that *ever* worked right. At
> least, it didn't in Tb 1.0 and hasn't in any 2006 build I tested. I can't find
> a specific bug for it, but it sounds like it's closely related to bug 228107.

Strange, that's also Mac related. Right clicking an attachment gives the focus to it and all entries inside the context menu are available.

Comment 50

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #48)
> Which is why it would be useful to be able to switch from within the context
> menu (comment #9 item 8). Should I file an RFE for this?

Peter, I will file appropriate bugs this evening. 

Comment 51

[Wayne Woods]

(In reply to comment #46)
> (From update of attachment 257348 [details] [diff] [review])
> fix checked in. Thanks Wayne!

Awesome. Thanks Scott :)

(In reply to comment #47)
> I don't mind checking in a height of 56 or 57 pixels on the Mac tonight, but I
> don't have access to a Mac right now so I can't verify that the fix will work.

I managed to test this out properly tonight, and I'm afraid I think we should leave the min-heights as Scott set them (sorry, Henrik). The reason is aesthetics, but the explanation is kind of lengthy...

30px and 60px are the lower height limits at which the attachments box will NOT overflow with one row of small or large icons, respectively (which I assume is why Scott chose those values). This doesn't stop us setting the min-height lower, as the attachment code should set it higher than that in most uses, anyway.

However there's an annoying bug that causes descriptionitems (attachments) to be calculated with a lower height the very first time they're loaded. And it seems to be attachment-specific, i.e. it'll occur for every new message with a unique attachment the first time it's loaded in a particular session, but not every subsequent time. What it means is that the first time a particular message is loaded, the attachments box is calculated to be shorter than it should be, and can result in the scrollbar being drawn when it isn't needed (i.e. for a one-row attachments box). The min-heights that Scott set prevent this happening and so hide the bug. If we set them even 1px lower, the unwanted scrollbars will appear the first time a message loads... and most people only load a message once in a session so that means the bug will be seen most times. Why this bug occurs, I have NO idea. I've tried to track it through the code, but still can't work it out. I know that it pre-dates the code added in bug 223340, but the effects were mostly masked. Whatever the cause, a fix probably won't make Tb 2.

Comment 52

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Missing focus when opening the context menu of an attachment

(In reply to comment #49)
> > The context menu appears to be coming from the attachments box and not the
> > descriptionitem icon. I actually wasn't aware that *ever* worked right. At
> > least, it didn't in Tb 1.0 and hasn't in any 2006 build I tested. I can't
> > find a specific bug for it, but it sounds like it's closely related to bug 228107.
> 
> Strange, that's also Mac related. Right clicking an attachment gives the focus
> to it and all entries inside the context menu are available.

Wayne, bug 228107 is for the compose window. I talk about the attachments in the mailwindow. This described issue does not occur on Windows. I can still reproduce it on Mac OS X. Have a look at the screenshot.

Comment 53

[Wayne Woods]

(In reply to comment #52)
> Wayne, bug 228107 is for the compose window. I talk about the attachments in
> the mailwindow.

Yeah, I wasn't saying it was the same bug. But it's the closest I could find as I suspect both relate to context-click not selecting the item before popping the context menu.

I added bug 372956 for this.

Comment 54

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

I'm guessing since the patches in this bug seem to have been checked in, the bug can be closed - shall we resolve and move on?

Comment 55

[Magnus Melin [:mkmelin]]

Sounds reasonable.