107707 - Collapsed header view doesn't show up (even after restart)

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P1)

Description

[Scott MacGregor]

Starting in today's release builds, if I change my message header view to
collapsed mode (which uses a grid), the header view area disappears and there's
no way to get the header pane to show the brief header mode anymore.

Nothing involving message header display changed on Friday. However, I noticed
that Eric turned on some new grid code. The collapsed header mode uses a grid. I
wonder if this is related. This problem appeared in the same build as the grid
changes. 

Eric can you help me out? 

Here's the grid that's used to implement brief mode. Do I need to change how my
grid is defined under your new grid code? It looks like we are giving zero space
for this grid now. Nothing gets painted.

<grid id="collapsedHeaderView" class="header-part1" flex="1" collapsed="true">
  <columns>
    <column class="collapsedToggleHdrBox" align="top" pack="center">
      <image id="toggleHeaderView" class="collapsedHeaderViewButton"
          onclick="ToggleHeaderView();"/>
    </column>
      
    <column id="collapsedsubjectBox" collapsed="true" crop="right" flex="1">
      <hbox>  
        <text class="collapsedHeaderDisplayName" value="&subjectField.label;"/>
        <text id="collapsedsubjectValue" class="collapsedHeaderValue"
crop="right" flex="1"/>
      </hbox>
	</column>

    <column id="collapsedfromBox" collapsed="true">
      <hbox>
        <text class="collapsedHeaderDisplayName" value="&fromField.label;"/>   
      
        <mail-emailaddress id="collapsedfromValue" flex="1"/>
      </hbox>
    </column>

    <column id = "collapseddateBox" collapsed="true">
      <hbox style="text-align: right;">
        <text id="collapseddateValue" class="collapsedHeaderValue"/>
      </hbox>
    </column>

    <column id="collapsedAttachmentBox" hide="true" >
      <image id="collapsedAttachment" class="collapsedAttachmentButton"
onclick="ToggleHeaderView();" />  
    </column>
  </columns>
</grid>

Comment 1

[Scott MacGregor]

adding some keyword pixie dust.

Comment 2

[scottputterman]

Adding more bug markings.  I just ran into this.  I had to edit my prefs.js and
localstore.rdf to get it back.

Comment 3

[R.K.Aa.]

*** Bug 107724 has been marked as a duplicate of this bug. ***

Comment 4

[R.K.Aa.]

Comment from dup:

"the same problem of lost headers occurs when you double click on a message to
view a full window version -- then you lose the headers in the full window view
for any subsiquent full window views."

Comment 5

[laurel]

*** Bug 107791 has been marked as a duplicate of this bug. ***

Comment 6

[Pratik]

Happens on linux too. OS should be All.

Comment 7

[Håkan Waara]

This blocks me from using recent builds. mscott, any response from evaughan yet?

Comment 8

[laurel]

*** Bug 107824 has been marked as a duplicate of this bug. ***

Comment 9

[Eric Vaughan]

This example does what it is supposed. You have collapsed="true" on the grid
that will collapse it completely. 

Other than that there was a bug in collapsing that I fixed. Try updating layout and 
rebuild.

Comment 10

[Asa Dotzler [:asa]]

I have no headers visible in mailnews. Is that this bug? I can't get a header
view back. If that isn't this bug can someone point me to the right bug please?
putterman, what specifically did you edit to get headers back? Do we need to
release note this for 0.9.6? Thanks.

Comment 11

[Scott MacGregor]

Yes Asa, that's this bug. 

evaughan emailed me this morning and said this was fixed by a checkin into
layout yesterday. I have not verified on a build from today to check if that's
the case though. No this shouldn't require a release note for 096 because this
would be an 096 stopper.

deleting your localstore.rdf should get you your headers back. Just don't go
into brief mode. 

Comment 12

[scottputterman]

if you want to keep your localstore.rdf for other reasons, what I did was open
up localstore.rdf, search for "header" and removed everything related to it(not
those referring to Custom Headers).

I agree with mscott.  If this hasn't been fixed, we can't ship 0.9.6 with this
since it's way to easy to get into a state with no headers.  We'd either have to
fix this or take out the collapsible feature.  Fixing it would be ideal :)

Comment 13

[sairuh (rarely reading bugmail)]

mscott: if something was checked in yesterday [10/30], it didn't fix this issue
--i still see this using 2001.10.31.08 bits. unless evaughan checked in stuff
today, post-8am. ;)

Comment 14

[Arthur Wagner]

Same problem for me. Last good build I can find is <2001102910>

Comment 15

[Arthur Wagner]

Modifying prefs.js or localstore.rdf don't seem to be logical. I can move back
and forth between <2001102910> and <2001103113> using the same prefs.js and
localstore.rdf while the problem comes<2001103113> and goes <2001102910>.
Might be related; setting of "view headers, all, normal" is very flaky.
I set it to "all" and the next time I look its back to "normal"

Comment 16

[Arthur Wagner]

Test results: This thing is a little wierd. I did some investigation by
switching back and forth between builds <20011029> and <20011031> and comparing
the contents  of prefs.js and localstore.rdf. The last time I went to <20011031>
the show headers [-] button appeared along with the header of the message!! I
clicked on the button and instesd of going to a [+] it totally disappeared never
to be found again.. I have tried going back and forth a few more times but it
hasen't shown it's head again.

Comment 17

[Asa Dotzler [:asa]]

this is not fixed in the morning or afternoon mozilla windows builds 10/31

Comment 18

[Matthias Versen [:Matti]]

*** Bug 108015 has been marked as a duplicate of this bug. ***

Comment 19

[Arthur Wagner]

Still seems to be in Build<2001110112>

Comment 20

[Reid Hekman]

Yup, still seen on Linux rpm 2001110111. Upon collapsing the header, there is a
small 1 or 2 pixel height black line in place of the usual "sparse" header --
this is different from the truly absent header area on the mail "welcome" screen.

Comment 21

[jay garcia]

Try this workaround: Doesn't work for me in today's afternoon build, Win98

To work around it, remove lines from your localstore.rdf:

<RDF:Description about="chrome://messenger/content/*.xul#msgHeaderView"
state="true" />

(*.xul, depends on if you use the normal or vertical 3 pane view)

Comment 22

[Alfonso Martinez]

*** Bug 108217 has been marked as a duplicate of this bug. ***

Comment 23

[Arthur Wagner]

The workaround above does NOT work for build <2001103113> Linux.
Is it because of different platforms, or two very similar bugs?
I have another possably related bug in the "view" - "headers" -
"all or normal" won't stay set to "all". Kind of a moot point since it
won'd display headers any way>

Comment 24

[Andreas Premstaller]

arthur, do you mean bug 67539? That one has a patch, hopefully Seth or someone
else from the mail-team will find the time to look at it before 11-07.

Comment 25

[Arthur Wagner]

No.. Andeas, I dont mean to say that. I just meant to point out that one group
of reports seem to be talking about a bug that can be affected by putzing with
prefs.js and or localstore.rdf and some others (like me) don't find that changing
those files have no affect at all. To add some credence to this I point out that
I can load up a pre 10/25/2001 build with the same pref file and .rdf file that
I use in failing builds, and the error follows only the build level regardless
of the contents of the files. 
As to the point I entered about the "view" - "header" - "normal or all", that
does still exist in <2001103113> (Linux) regardless BUG<67539>
Art 

Comment 26

[Arthur Wagner]

I am seeing something that MAY indicate this problem is some kind of a race
situation. As I reported before the header info pops up and then disapears
again.  Could there be a race condition between two operations that occur when
displaying a message? The pop up situation seems to occur when I have the
message window below the message list open. It does not seem to occur when I
have it (lower message window) closed and display by double clicking on the
message list, I notice the old "open mesage in new window" is gone.

Comment 27

[Jacek Piskozub]

> I notice the old "open mesage in new window" is gone.

I believe this is another bug #107377, fixed on 10/29. Anyway, I see this entry
on a fresh CVS build (timed around 2001110218)

Comment 28

[Olli Pettay [:smaug][[e-mail]]]

I changed <grid ...>...</grid> to <hbox ...>...</hbox> 
in msgHdrViewOverlay.xul -file, and this bug
was gone. 
Build Win32 2001110209.

Comment 29

[Olli Pettay [:smaug][[e-mail]]]

This is working even better.
Change the grid-area to this:

<hbox id="collapsedHeaderView" class="header-part1" collapsed="true" flex="1">

   <hbox class="collapsedToggleHdrBox" align="top" pack="center">
      <image id="toggleHeaderView" class="collapsedHeaderViewButton"
          onclick="ToggleHeaderView();"/>
   </hbox>
 
   <hbox id="collapsedsubjectBox" collapsed="true" crop="right" flex="1">  
     <text class="collapsedHeaderDisplayName" value="&subjectField.label;"/>
     <text id="collapsedsubjectValue" class="collapsedHeaderValue" crop="right" 
flex="1"/>
   </hbox>
	  
   <hbox id="collapsedfromBox" collapsed="true">
     <text class="collapsedHeaderDisplayName" value="&fromField.label;"/>       
   
     <mail-emailaddress id="collapsedfromValue" flex="1"/>
   </hbox>
 
   <hbox style="text-align: right;"  id = "collapseddateBox" collapsed="true">
     <text id="collapseddateValue" class="collapsedHeaderValue"/>
   </hbox>
   
   <hbox id="collapsedAttachmentBox" hide="true" >
      <image id="collapsedAttachment" class="collapsedAttachmentButton" 
onclick="ToggleHeaderView();" />  
   </hbox>

</hbox>

Comment 30

[[not reading bugmail]]

so then there are issues with grid code from the checkin?

Comment 31

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Should the grid have a 'rows' and a 'row' element? BTW. In expanded view there is a bug with very long subject field: the message header becomes too wide.

Comment 32

[Olli Pettay [:smaug][[e-mail]]]

I made a patch for this bug. 
It also corrects a bug when message contains
very long subject field.
I hope this works ok.

Comment 33

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Patchfile 1/2 (mailWidgets.xml)

Comment 34

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Patchfile 2/2 (msgHdrViewOverlay.xul)

Comment 35

[Håkan Waara]

Smaug, please attach a patch - not a whole file so we can apply it to our trees 
and review...  If you don't have cvs, and create a cvs diff -u patch, consider 
downloading Patch Maker (search on it on mozilla.org) which was created just 
for patches like yours...

Comment 36

[Alfonso Martinez]

*** Bug 108454 has been marked as a duplicate of this bug. ***

Comment 37

[Brian S. Craigie]

Just an observation, but can't we make the header pane scroll with the message
window?  That way, if the headers take up all the message window, we can just
scroll down to the message.
Another useful option would be to be able to set view headers to Normal, yet
have a 'forward with headers' button/menu.  Just an idea.

Comment 38

[Olli Pettay [:smaug][[e-mail]]]

Just installed the Patch Maker.
I'll attach the diff/patch.

It corrects only this 107707 bug,
not the one with long
subject-fields.

Comment 39

[Olli Pettay [:smaug][[e-mail]]]

Attachment: This is the diff-file for bug 107707.

Comment 40

[Aleksander Adamowski]

brianc: there is a bug about scrollable message headers. It's bug 9942.

Comment 41

[laurel]

*** Bug 108529 has been marked as a duplicate of this bug. ***

Comment 42

[Olga]

*** Bug 108113 has been marked as a duplicate of this bug. ***

Comment 43

[Olli Pettay [:smaug][[e-mail]]]

Has anyone tried my patch, is it ok?
This is my first try
to fix a mozilla's bug, so I'm
not so sure how to proceed.

PS. I don't have CVS access

Comment 44

[Ben Ruppel]

Hi Smaug, I tried your patch and it looks good on the surface (I didn't actually
look at the code).  One problem is that the "From:" field in the collapsed
header view is centered, where I think it used to be further to the right. 
Second, and this problem may have existed before, is that when I have headers
set to "all", the bottom line seems misaligned.  For instance, in a bugmail, the
bottom line is:
"X-Bugzilla-Reason: Voter"
Voter is aligned lower than X-Bugzilla-Reason:
This may be totally unrelated though.

I'm on win32 using a November 1st build.  Can someone check this out and get it
checked it so recent nightlies won't suck so much?

Comment 45

[Scott MacGregor]

Attachment: the fix: add a row attribute to the grid

Comment 46

[Scott MacGregor]

Thanks for the patch smaug, I just attached a  different, more minimal patch
which also fixes/works around the grid problem. But your patch showed me what
the problem was so it was still very helpful.

This will go into the tree tonight so brief header mode will start working again.

Comment 47

[Scott MacGregor]

Comment on attachment 56762 [details] [diff] [review]
the fix: add a row attribute to the grid

I got an sr=sspitzer on this.

Comment 48

[Scott MacGregor]

fix checked in.

Comment 49

[Eric Vaughan]

Attachment: This fixes the bug in grid, so there is no need to change the xul. Headers work just fine in my build now.

Comment 50

[Eric Vaughan]

Reopening and assigning to myself. While the patch fixes mail it does not 
address the underlying problem in grid that this patch fixes.

Comment 51

[Eric Vaughan]

Reassigning

Comment 52

[Eric Vaughan]

Attachment: Better patch.

Comment 53

[jag (Peter Annema)]

Comment on attachment 56835 [details] [diff] [review]
Better patch.

>Index: nsGridLayout2.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/layout/xul/base/src/grid/nsGridLayout2.cpp,v
>retrieving revision 1.4
>diff -u -r1.4 nsGridLayout2.cpp
>--- nsGridLayout2.cpp	2001/10/26 02:41:05	1.4
>+++ nsGridLayout2.cpp	2001/11/07 01:53:10
>@@ -88,22 +88,146 @@
>   return NS_ERROR_FAILURE;
> }
> 
>+void
>+nsGridLayout2::AddWidth(nsSize& aSize, nscoord aSize2, PRBool aIsHorizontal)
>+{
>+  nscoord& size = GET_WIDTH(aSize, aIsHorizontal);
>+
>+  if (size == NS_INTRINSICSIZE || aSize2 == NS_INTRINSICSIZE)
>+    size = NS_INTRINSICSIZE;

Technically the first part of that condition isn't needed.

> NS_IMETHODIMP
> nsGridLayout2::GetMinSize(nsIBox* aBox, nsBoxLayoutState& aState, nsSize& aSize)
> {
>-  return nsStackLayout::GetMinSize(aBox, aState, aSize);
>+  nsresult rv = nsStackLayout::GetMinSize(aBox, aState, aSize); 

Should the code below be guarded on the value of rv? E.g.:

    if (NS_FAILED(rv))
      return rv;

>+  // if there are no <rows> tags that will sum up our columns,
>+  // sum up our columns here.

...

>+        AddWidth(total,size, PR_TRUE); // AddWidth

Add a space before |size| (times 2).

Same comments for GetPrefSize and GetMaxSize.


>Index: nsGridRowGroupLayout.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/layout/xul/base/src/grid/nsGridRowGroupLayout.cpp,v
>retrieving revision 1.5
>diff -u -r1.5 nsGridRowGroupLayout.cpp
>--- nsGridRowGroupLayout.cpp	2001/10/30 23:41:30	1.5
>+++ nsGridRowGroupLayout.cpp	2001/11/07 01:53:10
>@@ -156,7 +156,7 @@
>       grid->GetMaxRowHeight(aState, i+start, size, !isHorizontal); // GetMaxColumnWidth
> 
>       AddWidth(aSize, size, isHorizontal);
>-    }
>+    } 

You added a space after that }, can get rid of that change :-)

>Index: nsGridRowLayout.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/layout/xul/base/src/grid/nsGridRowLayout.cpp,v
>retrieving revision 1.3
>diff -u -r1.3 nsGridRowLayout.cpp
>--- nsGridRowLayout.cpp	2001/10/26 02:41:06	1.3
>+++ nsGridRowLayout.cpp	2001/11/07 01:53:10
>@@ -250,7 +250,6 @@
>   // add ours to it.
>   aBox->GetMargin(margin);
>   aMargin += margin;
>-
>   return NS_OK;
> }
> 

Don't need to make this change either.

Comment 54

[Eric Vaughan]

>+{
>+  nscoord& size = GET_WIDTH(aSize, aIsHorizontal);
>+
>+  if (size == NS_INTRINSICSIZE || aSize2 == NS_INTRINSICSIZE)
>+    size = NS_INTRINSICSIZE;

the first part is necessary. It will fall into the else statement below this 
otherwise and try to add a NS_INTRINSICSIZE which will overflow the integer.

I could just put an if around this that says:

if (size != NS_INTRINSICSIZE) {

}

That might be clearer.

Comment 55

[Eric Vaughan]

Attachment: patch with fixes.

Comment 56

[R.K.Aa.]

*** Bug 108954 has been marked as a duplicate of this bug. ***

Comment 57

[David Hyatt]

sr=hyatt

Comment 58

[jag (Peter Annema)]

Comment on attachment 56918 [details] [diff] [review]
patch with fixes.

r=jag

Comment 59

[Asa Dotzler [:asa]]

a=asa (on behalf of drivers) for checkin to 0.9.6

Comment 60

[Scott MacGregor]

my work around is already 0.9.6 so we probably don't need the change to grid for
the 096 branch but for the trunk only. my  2 cents.

Comment 61

[Silviu Marin-Caea]

I tested the 2001-11-08 build, and the bug is resolved, but not quite perfectly.

If you use the Classic theme, clicking on [-] left of Subject still makes all
headers disappear, but if you move to another message, and then back, the line
with [+] Subject reappears.

Comment 62

[Matthew Gifford]

It does not seem to be resolved in build 2001110803 using the Modern theme. 
Removing the thread column leaves message header information misaligned. 
Clicking on a message header will realign the information.  Going to another
folder will cause header information in that folder or any subsequent folder to
not appear.

Comment 63

[Eric Vaughan]

fixed

Comment 64

[Boris Zbarsky [:bzbarsky]]

*** Bug 109197 has been marked as a duplicate of this bug. ***

Comment 65

[laurel]

*** Bug 109337 has been marked as a duplicate of this bug. ***

Comment 66

[David A. Cobb]

Yes, verify fixed in 2001-11-09-03.

Comment 67

[Thommie Rother]

Verified fixed in Mozilla 0.9.5+
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.5+) Gecko/20011110

Comment 68

[laurel]

OK using commercial trunk build: mac OS X, win98, linux rh6.2

Comment 69

[Christopher Blizzard (:blizzard)]

Has this been checked into the 0.9.6 branch?

Comment 70

[Christopher Blizzard (:blizzard)]

Ahh, so it has.  Nevermind.