Multiple references in message header causes scrollbar issues

RESOLVED FIXED in Thunderbird 3.0rc1

Status

Thunderbird
Message Reader UI
--
trivial
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: squib, Unassigned)

Tracking

unspecified
Thunderbird 3.0rc1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

8 years ago
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.
(Reporter)

Comment 1

8 years ago
Created attachment 396029 [details]
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.
Whiteboard: dupeme
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 3

8 years ago
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.
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?
(Reporter)

Comment 5

8 years ago
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.
(Reporter)

Comment 6

8 years ago
Created attachment 396814 [details] [diff] [review]
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. :)
Attachment #396814 - Flags: review?(dmose)
(Reporter)

Comment 7

8 years ago
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.
Attachment #396814 - Attachment is obsolete: true
Attachment #396814 - Flags: review?(dmose)
(Reporter)

Comment 8

8 years ago
Created attachment 396821 [details]
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.
(Reporter)

Comment 9

8 years ago
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.
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...
(Reporter)

Updated

8 years ago
Depends on: 513318
(Reporter)

Comment 11

8 years ago
I've filed bug 513318 for the Core issue. Time to wait and see what the response is.
No longer depends on: 513318
(Reporter)

Updated

8 years ago
Depends on: 513318

Comment 12

8 years ago
Adding bug 498590 as dependency as it's quite similar, i suspect a dupe in the end.
Depends on: 498590
Fixed by 1.9.1 landing of bug 513318.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
OS: Windows XP → All
Hardware: x86 → All
Resolution: --- → FIXED
Whiteboard: dupeme
Target Milestone: --- → Thunderbird 3.0rc1
You need to log in before you can comment on or make changes to this bug.