512036 - Multiple references in message header causes scrollbar issues

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, defect)

Description

[Jim Porter (:squib)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.2) Gecko/20090729 Firefox/3.5.2 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.3pre) Gecko/20090821 Shredder/3.0b4pre

After viewing a message with multiple references displayed in its header, all subsequent messages will have extra space at the bottom of the message header, along with a scrollbar. This can only be reset by either clicking on a message with a single reference, or restarting Thunderbird.

The amount of empty space is directly proportional to the number of commas in the original message's References header. Looking at the header with the DOM inspector reveals that the XUL element for each comma is stacked vertically, but its contents are not rendered.

(As a possibly related issue, a scrollbar appears whenever the Reference header is displayed, but will go away if you go from a message with 1 reference to a message with no references.)

Reproducible: Always

Steps to Reproduce:
1. Click on a message (e.g. news) with multiple references
2. Click to a message with no references

Actual Results:  
Lots of empty space in the message header.

Expected Results:  
Well... no empty space!

A quick-and-dirty solution is to set display:none; whenever the "collapsed" attribute on the mail-messageids-headerfield is true, but that's probably the Wrong Way.

Comment 1

[Jim Porter (:squib)]

Attachment: Exampe of issue

Before/after view of message header. Note that the there are 4 commas in the top image, and that the empty space in the bottom image is roughly 4em high.

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

I've seen this before, and I hadn't figured out what was causing it; nice work tracking it down!  

Any idea _why_ they're stacked vertically?

My suspicion is that whatever the right fix is, it's likely to end up in mailWidgets.xml -> mail-messageids-headerfields -> clearValues.

Comment 3

[Jim Porter (:squib)]

Ok, based on my reading of the visibility CSS property on MDC, this *might* be a XUL bug. Under the section for the collapse value, it says "For XUL elements, the computed size of the element is always zero", but the DOM inspector shows the text nodes as having non-zero size. The other nodes (mail-messageid) have zero size as you'd expect, however.

The only thing I can think of is that the text nodes aren't being treated as XUL elements, since this is basically what should happen for non-XUL nodes (they act as though you specified visibility:hidden, and so still take up space).

That still doesn't explain why they get stacked vertically instead of horizontally, but that could be some strange interaction between -moz-box elements and visibility:collapse.

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

I can imagine two ways forward with this:

* try and come up with a reduced test case for the XUL bug and try and get that fixed
* run with the quick-n-dirty fix you suggested

At this late stage of the Tb3 cycle, I think the quick-n-dirty fix would be entirely reasonable.  Are you interested in taking a run at either of these options?

Comment 5

[Jim Porter (:squib)]

Well, I haven't been able to create a test case for this yet, despite trying for a while. In most simple cases, it just works, but maybe there's something strange happening with the interaction of overlays, XBL, and collapsing that's causing this.

I did find one interesting trait, however: the height of the collapsed element doesn't push other elements out of the way. It only affects the height of the containing element.

Comment 6

[Jim Porter (:squib)]

Attachment: Fix scrollbars

This fixes the extra space at the bottom of the message header (the second picture in my original attachment) as well as the unnecessary scrollbar that appears when references are displayed (the first picture). I'm not entirely sure *why* it fixes the problem, but it does. :)

Comment 7

[Jim Porter (:squib)]

Comment on attachment 396814 [details] [diff] [review]
Fix scrollbars

Actually, nevermind on that patch. It'll cause issues if the list of references is long enough to wrap.

Comment 8

[Jim Porter (:squib)]

Attachment: Minimal test case

Here's a test case that shows the problem in action. The fact that I used text nodes doesn't matter; any XUL element will suffice. However, it *is* necessary to have the nodes inside a label (or something similar, like a description) inside of an hbox. Collapsing the label instead of the hbox also produces the same behavior.

Comment 9

[Jim Porter (:squib)]

One last note of interest: this does not appear to happen with XBL-bound elements (which is why mail-messageid elements don't contribute to the height when collapsed). This suggests that one solution is to make an XBL binding called "comma" and use that instead.

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

Nice work!  I think the right thing to do is to spin off the minimal test case into a new bug in Product: Core; Component: Layout, and mark that bug as blocking this one.  It'd be reasonable to wait for a little bit to see if that gets traction, and if not, just go ahead with a workaround (eg the comma binding) in this bug.  If you CC me on the Layout bug, I suspect I can convince one of the layout hackers to look at it fairly quickly...

Comment 11

[Jim Porter (:squib)]

I've filed bug 513318 for the Core issue. Time to wait and see what the response is.

Comment 12

[Magnus Melin [:mkmelin]]

Adding bug 498590 as dependency as it's quite similar, i suspect a dupe in the end.

Comment 13

[Phil Ringnalda (:philor)]

Fixed by 1.9.1 landing of bug 513318.