105542 - dynamically generate 3 pane layout, so we can get rid of the alt3-pane xul

Status: RESOLVED FIXED

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

Description

[(not reading, please use [e-mail] instead)]

dynamically generate 3 pane layout, so we can get rid of the alt3-pane xul

I just want one .xul for the 3 pane, messenger.xul.  maintaining the alternate 
3 pane xul is a pain, and we often forget about it, or updating localstore.rdf 
for both messenger and the vert layout.

so the primary reason for this is to make our code more maintainable.

but it would solve the bugs where if you hide a column in one layout, and then 
switch to another, the column comes back.

this would also allow user to dynamically change the 3 pane layout, and not 
have changes take affect after restart.

I'm guessing that in the onload handler for messenger.xul, we'd dynamically 
poke the content, based on the layout pref, and when the pref changes, poke the 
content again.

I'll investigate.

Comment 1

[(not reading, please use [e-mail] instead)]

timeless, do you know the bug where someone is trying to clean up, simplify the 
xul between messenger.xul and the alternate 3 pane xul file?

if this approach will work, that bug will become invalid.

Comment 2

[timeless]

Bugzilla Bug 66475 

Comment 3

[[e-mail]]

Aha! You can put everything into View/Folder Display/
* Above sidebar
* Above message
* Hidden
thus resolving two bugs with one patch.

Mind you, there's still some cleanup possible - some of the context menus belong
in msgHdrViewOverlay.xul and other folder and threadpane stuff could also be
moved out of mailWindowOverlay.xul

Comment 4

[[e-mail]]

Obviously this needs to be properly integrated into the sidebar and preferences,
but I added this code to mailWindowOverlay.xul and starting with the alternate
layout successfully changed the layout between the three alternatives above.

<menu label="Folders" accesskey="F">
  <menupopup onpopupshowing="
    var folderpane = document.getElementById('folderPaneBox');
    this.childNodes[0].removeAttribute('checked');
    this.childNodes[1].removeAttribute('checked');
    this.childNodes[2].removeAttribute('checked');
    if (folderpane.getAttribute('collapsed') != 'true')
      this.childNodes[1].setAttribute('checked', 'true');
    else if (folderpane.firstChild)
      this.childNodes[2].setAttribute('checked', 'true');
    else
      this.childNodes[0].setAttribute('checked', 'true');
    ">
    <menuitem type="radio" label="Above Sidebar" accesskey="S" oncommand="
      var folderpane = document.getElementById('folderPaneBox');
      var sidebarbox = document.getElementById('sidebar-box');
      if (folderpane.firstChild)
        sidebarbox.insertBefore(folderpane.firstChild, sidebarbox.firstChild);
      sidebarbox.childNodes[1].removeAttribute('hidden');
      folderpane.setAttribute('collapsed', 'true');
      folderpane.nextSibling.setAttribute('collapsed', 'true');
      sidebarbox.removeAttribute('collapsed');
      "/>
    <menuitem type="radio" label="Above Message" accesskey="M" oncommand="
      var folderpane = document.getElementById('folderPaneBox');
      var sidebarbox = document.getElementById('sidebar-box');
      if (!folderpane.firstChild)
        folderpane.appendChild(sidebarbox.firstChild);
      sidebarbox.firstChild.setAttribute('hidden', 'true');
      folderpane.removeAttribute('collapsed');
      folderpane.nextSibling.removeAttribute('collapsed');
      folderpane.nextSibling.removeAttribute('state');
      "/>
    <menuitem type="radio" label="Hidden" accesskey="H" oncommand="
      var folderpane = document.getElementById('folderPaneBox');
      var sidebarbox = document.getElementById('sidebar-box');
      if (!folderpane.firstChild)
        folderpane.appendChild(sidebarbox.firstChild);
      sidebarbox.firstChild.setAttribute('hidden', 'true');
      folderpane.setAttribute('collapsed', 'true');
      folderpane.nextSibling.setAttribute('collapsed', 'true');
      "/>
  </menupopup>
</menu>

Comment 5

[Dimitrios]

I hope these might add some value to the bug (if related):
-After installing build 2001102403, I saw the header part of the alt 3pane
window narrowed to the point where neither headers nor attachments were visible.
Reverting to 0.9.5 milestone corrected the problem. Reinstalling 2001102403,
problem re-appeared again. I was about to file a bug but, after changing to
normal 3 pane view and going again to alt 3 pane, problem solved.
-I need to restart in order to change alt to normal pane view and vice versa.

Comment 6

[[e-mail]]

Attachment: Basic dynamic support

Comment 7

[[e-mail]]

This patch allows the mail window to switch the pane config dynamically.
The references to mail3PaneWindowVertLayout.xul still need to be removed.

Comment 8

[[e-mail]]

Attachment: Final patch

Comment 9

[[e-mail]]

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

Comment 10

[(not reading, please use [e-mail] instead)]

moving back to 0.9.9

I don't have the cycles to test / review this right now (as we are in the
performance lock down) but this would be a HUGE win for mailnews.

I'll get to it as soon as I can.

thanks for the patch, neil.

Comment 11

[Dimitrios]

I see bug 83366 appearing again in last week's builds (at least, a variant of
it). Should I re-open bug 833366 and set it blocked by 105542 or it is better to
wait until you fix this one ?

Comment 12

[scottputterman]

moving to 1.0.1

Comment 13

[[e-mail]]

[e-mail] wrote:

> moving to 1.0.1

Why? There's a patch, it's just waiting for the team to have time to review it.

Comment 14

[[e-mail]]

Attachment: Original patch updated for bitrot

This patch only updates files in messenger content.

Comment 15

[(not reading, please use [e-mail] instead)]

the last patch has js errors.  neil, can you double check your if / else logic 
in UpdateMailPaneConfig()

Comment 16

[[e-mail]]

Attachment: Added missing }

Comment 17

[[e-mail]]

Attachment: Fix for collapsed folder pane

This patch fixes problems with starting Mail with a collapsed folder pane, and
fixes problems with the previous patch not persisting some attributes properly.

Comment 18

[(not reading, please use [e-mail] instead)]

I tried out the patch, it might have bit rotted (which would be my fault since 
I'm overloaded and getting to this patch later, rather than sooner.  I 
apologize.)

I'm seeing some errors to the console:

failed to cycle header: TypeError: folderOutliner.outlinerBoxObject.view has no
properties
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && window) failed, file c:\builds\bridg
e\mozilla\content\xul\document\src\nsXULCommandDispatcher.cpp, line 157
************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'[JavaScript Error: "folderOutliner.outlinerBoxObject.selection h
as no properties" {file: "chrome://messenger/content/msgMail3PaneWindow.js" line
: 1033}]' when calling method: [nsIController::isCommandEnabled]"  nsresult: "0x
80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: c
hrome://global/content/globalOverlay.js :: goUpdateCommand :: line 53"  data: ye
s]
************************************************************
An error occurred updating the button_getNewMessages command
************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'[JavaScript Error: "selection has no properties" {file: "chrome:
//messenger/content/mail3PaneWindowCommands.js" line: 892}]' when calling method
: [nsIController::isCommandEnabled]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASC
RIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: chrome://global/content/global
Overlay.js :: goUpdateCommand :: line 53"  data: yes]
************************************************************
An error occurred updating the button_next command


My inbox was also in the wrong place, I'm not sure why that would be.

On toggling the pref, I crashed:

nsQueryInterface::operator()(const nsID & {...}, void * * 0x0012e920) line 47 + 
23 bytes
nsCOMPtr<nsIBoxObject>::assign_from_helper(const nsCOMPtr_helper & {...}, const 
nsID & {...}) line 972 + 18 bytes
nsCOMPtr<nsIBoxObject>::nsCOMPtr<nsIBoxObject>(const nsQueryInterface & {...}) 
line 566
nsOutlinerSelection::FireOnSelectHandler() line 714
nsOutlinerSelection::AdjustSelection(nsOutlinerSelection * const 0x04c4b970, 
int 0, int 45) line 689
nsOutlinerBodyFrame::RowCountChanged(nsOutlinerBodyFrame * const 0x0367f9a8, 
int 0, int 45) line 1496
nsOutlinerBoxObject::RowCountChanged(nsOutlinerBoxObject * const 0x045e0030, 
int 0, int 45) line 393 + 20 bytes
nsXULOutlinerBuilder::OpenContainer(int -1, nsIRDFResource * 0x035706e0) line 
1489
nsXULOutlinerBuilder::Rebuild(nsXULOutlinerBuilder * const 0x03d69dc0) line 987
nsXULOutlinerBuilder::SetOutliner(nsXULOutlinerBuilder * const 0x03d69ea8, 
nsIOutlinerBoxObject * 0x045e0030) line 693
nsOutlinerBodyFrame::SetView(nsOutlinerBodyFrame * const 0x0367f9a8, 
nsIOutlinerView * 0x03d69ea8) line 637
nsOutlinerBodyFrame::GetPrefSize(nsOutlinerBodyFrame * const 0x0367f998, 
nsBoxLayoutState & {...}, nsSize & {...}) line 428
nsBox::GetAscent(nsBox * const 0x0367f998, nsBoxLayoutState & {...}, int & 0) 
line 1031 + 20 bytes
nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x02d91220, nsIBox * 
0x0367f774, nsBoxLayoutState & {...}, int & 0) line 1520
nsContainerBox::GetAscent(nsContainerBox * const 0x0367f774, nsBoxLayoutState & 
{...}, int & 0) line 590 + 38 bytes
nsBoxFrame::GetAscent(nsBoxFrame * const 0x0367f774, nsBoxLayoutState & {...}, 
int & 0) line 1092 + 20 bytes
nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x02d91220, nsIBox * 
0x0367f314, nsBoxLayoutState & {...}, int & 0) line 1520
nsContainerBox::GetAscent(nsContainerBox * const 0x0367f314, nsBoxLayoutState & 
{...}, int & 0) line 590 + 38 bytes
nsBoxFrame::GetAscent(nsBoxFrame * const 0x0367f314, nsBoxLayoutState & {...}, 
int & 0) line 1092 + 20 bytes
nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x02d91220, nsIBox * 
0x03625c14, nsBoxLayoutState & {...}, int & 165) line 1520
nsContainerBox::GetAscent(nsContainerBox * const 0x03625c14, nsBoxLayoutState & 
{...}, int & 165) line 590 + 38 bytes
nsBoxFrame::GetAscent(nsBoxFrame * const 0x03625c14, nsBoxLayoutState & {...}, 
int & 0) line 1092 + 20 bytes
nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x02d91220, nsIBox * 
0x035e8744, nsBoxLayoutState & {...}, int & 0) line 1520
nsContainerBox::GetAscent(nsContainerBox * const 0x035e8744, nsBoxLayoutState & 
{...}, int & 0) line 590 + 38 bytes
nsBoxFrame::GetAscent(nsBoxFrame * const 0x035e8744, nsBoxLayoutState & {...}, 
int & 0) line 1092 + 20 bytes
nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x02d91220, nsIBox * 
0x035e8624, nsBoxLayoutState & {...}, int & 0) line 1520
nsContainerBox::GetAscent(nsContainerBox * const 0x035e8624, nsBoxLayoutState & 
{...}, int & 0) line 590 + 38 bytes
nsBoxFrame::GetAscent(nsBoxFrame * const 0x035e8624, nsBoxLayoutState & {...}, 
int & 0) line 1092 + 20 bytes
nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x02d91220, nsIBox * 
0x035e853c, nsBoxLayoutState & {...}, int & 0) line 1520
nsContainerBox::GetAscent(nsContainerBox * const 0x035e853c, nsBoxLayoutState & 
{...}, int & 0) line 590 + 38 bytes
nsBoxFrame::GetAscent(nsBoxFrame * const 0x035e853c, nsBoxLayoutState & {...}, 
int & 0) line 1092 + 20 bytes
nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x02d91220, nsIBox * 
0x035b2cd4, nsBoxLayoutState & {...}, int & 180) line 1520
nsContainerBox::GetAscent(nsContainerBox * const 0x035b2cd4, nsBoxLayoutState & 
{...}, int & 180) line 590 + 38 bytes
nsBoxFrame::GetAscent(nsBoxFrame * const 0x035b2cd4, nsBoxLayoutState & {...}, 
int & 0) line 1092 + 20 bytes
nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x02d91220, nsIBox * 
0x03584a40, nsBoxLayoutState & {...}, int & 180) line 1520
nsContainerBox::GetAscent(nsContainerBox * const 0x03584a40, nsBoxLayoutState & 
{...}, int & 180) line 590 + 38 bytes
nsBoxFrame::GetAscent(nsBoxFrame * const 0x03584a40, nsBoxLayoutState & {...}, 
int & 0) line 1092 + 20 bytes
nsSprocketLayout::Layout(nsSprocketLayout * const 0x02d91220, nsIBox * 
0x03584a40, nsBoxLayoutState & {...}) line 242
nsContainerBox::DoLayout(nsContainerBox * const 0x03584a40, nsBoxLayoutState & 
{...}) line 606 + 34 bytes
nsBoxFrame::DoLayout(nsBoxFrame * const 0x03584a40, nsBoxLayoutState & {...}) 
line 1201
nsBox::Layout(nsBox * const 0x03584a40, nsBoxLayoutState & {...}) line 1052
nsStackLayout::Layout(nsStackLayout * const 0x03dbaec0, nsIBox * 0x03584840, 
nsBoxLayoutState & {...}) line 331
nsContainerBox::DoLayout(nsContainerBox * const 0x03584840, nsBoxLayoutState & 
{...}) line 606 + 34 bytes
nsBoxFrame::DoLayout(nsBoxFrame * const 0x03584840, nsBoxLayoutState & {...}) 
line 1201
nsBox::Layout(nsBox * const 0x03584840, nsBoxLayoutState & {...}) line 1052
nsBoxFrame::Reflow(nsBoxFrame * const 0x03584808, nsIPresContext * 0x02dd0d90, 
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) 
line 994
nsRootBoxFrame::Reflow(nsRootBoxFrame * const 0x03584808, nsIPresContext * 
0x02dd0d90, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, 
unsigned int & 0) line 243
nsContainerFrame::ReflowChild(nsIFrame * 0x03584808, nsIPresContext * 
0x02dd0d90, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 
0, int 0, unsigned int 0, unsigned int & 0) line 768 + 31 bytes
ViewportFrame::Reflow(ViewportFrame * const 0x035846e4, nsIPresContext * 
0x02dd0d90, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, 
unsigned int & 0) line 574
nsHTMLReflowCommand::Dispatch(nsIPresContext * 0x02dd0d90, nsHTMLReflowMetrics 
& {...}, const nsSize & {...}, nsIRenderingContext & {...}) line 217
PresShell::ProcessReflowCommand(nsVoidArray & {...}, int 1, nsHTMLReflowMetrics 
& {...}, nsSize & {...}, nsIRenderingContext & {...}) line 6205
PresShell::ProcessReflowCommands(int 1) line 6260
ReflowEvent::HandleEvent() line 6116
HandlePLEvent(ReflowEvent * 0x06198580) line 6130
PL_HandleEvent(PLEvent * 0x06198580) line 590 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x004a8730) line 520 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x001a0218, unsigned int 49391, unsigned int 0, 
long 4884272) line 1071 + 9 bytes
USER32! 77e13eb0()
USER32! 77e1401a()
USER32! 77e192da()
nsAppShellService::Run(nsAppShellService * const 0x01d15b70) line 308
main1(int 2, char * * 0x00444b90, nsISupports * 0x00000000) line 1285 + 32 bytes
main(int 2, char * * 0x00444b90) line 1625 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e87903()

Another issue is that after applying the patch, as you'd expect, what I was 
persisting in my localstore.rdf was no longer valid, so:

1)  my thread pane height, folder pane width changed
2)  my sidebar opened up

Neil, are you seeing any of these issues?

Comment 19

[timeless]

I've never played with this patch, so my comments are merely about the errors 
sspitzer encountered.

> WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && window) failed, file
> mozilla\content\xul\document\src\nsXULCommandDispatcher.cpp, line 157
I get that all the time. I intend to find out why, but haven't had the time :-(

> An error occurred updating the button_getNewMessages command
I had this in some recent builds when i had a hacked prompt service (it had a 
broken QueryInterface function -- actually it had a queryInterface and no 
QueryInterface).

> An error occurred updating the button_next command
i've seen this too, but I can't remember where or why

> My inbox was also in the wrong place, I'm not sure why that would be.
I've had my news and mail servers jockey for top position (it seems to change 
with each launch), is that what you mean?

[@nsOutlinerSelection::FireOnSelectHandler() line 714] jan: any idea how this 
could happen?

1/2 would be expected. Do they persist after you've set them in the new world?

fwiw, mailnews is generally abusive of outliner :-) see all the bugs i've filed 
lately with outliner in the summary.

Comment 20

[[e-mail]]

Seth,
Patch 66725 has those errors, because someone "optimized" the outliner so you
can't do anything useful like sort it until it's visible :-( Patch 68867 fixes that.

As for the persistence in localstore.rdf, unfortunately I can't arrange to
import the existing peristence, to allow for mode switch with and without an
open window.

Comment 21

[Jan Varga [:janv]]

apropo "outliner optimization":
All outliner properties are only available when outliner body frame is available,
since all properties are stored in outliner body frame class.

Comment 22

[(not reading, please use [e-mail] instead)]

moving to 1.01, 

there are issues with this, and the return on investment for this bug doesn't 
outweigh my other 0.9.9 and 1.0 bugs (my 1.0 list is big, we're still triaging 
it.)

sorry neil, this will have to wait.

Comment 23

[Olga]

I remove nsbeta1- keyword - it was for MachV.

Comment 24

[Olga]

Now I add 'nsbeta1' keyword for Buffy. Sorry, it's faster to edit multiple bugs
at once than manually go to each and remove minus.

Comment 25

[(not reading, please use [e-mail] instead)]

I tried to resurrect this patch, I'll attach what I got.

but I kept crashing on pane config:

nsQueryInterface::operator()(const nsID & {...}, void * * 0x0012e9d0) line 47 + 
23 bytes
nsCOMPtr<nsIBoxObject>::assign_from_helper(const nsCOMPtr_helper & {...}, const 
nsID & {...}) line 922 + 18 bytes
nsCOMPtr<nsIBoxObject>::nsCOMPtr<nsIBoxObject>(const nsQueryInterface & {...}) 
line 566
nsTreeSelection::FireOnSelectHandler() line 715
nsTreeSelection::AdjustSelection(nsTreeSelection * const 0x03bb7fa0, int 0, int 
64) line 690
nsTreeBodyFrame::RowCountChanged(nsTreeBodyFrame * const 0x03978cc0, int 0, int 
64) line 1660
nsTreeBoxObject::RowCountChanged(nsTreeBoxObject * const 0x03977478, int 0, int 
64) line 424 + 20 bytes
nsXULTreeBuilder::OpenContainer(int -1, nsIRDFResource * 0x02ddcb80) line 1578
nsXULTreeBuilder::Rebuild(nsXULTreeBuilder * const 0x0307d878) line 1075
nsXULTreeBuilder::SetTree(nsXULTreeBuilder * const 0x0307d960, nsITreeBoxObject 
* 0x03977478) line 834
nsTreeBodyFrame::SetView(nsTreeBodyFrame * const 0x03978cc0, nsITreeView * 
0x0307d960) line 710
nsTreeBodyFrame::EnsureView() line 607
nsTreeBodyFrame::GetPrefSize(nsTreeBodyFrame * const 0x03978cb0, 
nsBoxLayoutState & {...}, nsSize & {...}) line 397
nsBox::GetAscent(nsBox * const 0x03978cb0, nsBoxLayoutState & {...}, int & 0) 
line 1043 + 20 bytes
nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x02d27210, nsIBox * 
0x0397847c, nsBoxLayoutState & {...}, int & 0) line 1520
nsContainerBox::GetAscent(nsContainerBox * const 0x0397847c, nsBoxLayoutState & 
{...}, int & 0) line 591 + 38 bytes
nsBoxFrame::GetAscent(nsBoxFrame * const 0x0397847c, nsBoxLayoutState & {...}, 
int & 0) line 1106 + 20 bytes
nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x02d27210, nsIBox * 
0x03978844, nsBoxLayoutState & {...}, int & 0) line 1520
nsContainerBox::GetAscent(nsContainerBox * const 0x03978844, nsBoxLayoutState & 
{...}, int & 0) line 591 + 38 bytes
nsBoxFrame::GetAscent(nsBoxFrame * const 0x03978844, nsBoxLayoutState & {...}, 
int & 0) line 1106 + 20 bytes
nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x02d27210, nsIBox * 
0x0396238c, nsBoxLayoutState & {...}, int & 165) line 1520
nsContainerBox::GetAscent(nsContainerBox * const 0x0396238c, nsBoxLayoutState & 
{...}, int & 165) line 591 + 38 bytes
nsBoxFrame::GetAscent(nsBoxFrame * const 0x0396238c, nsBoxLayoutState & {...}, 
int & 0) line 1106 + 20 bytes
nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x02d27210, nsIBox * 
0x03649820, nsBoxLayoutState & {...}, int & 0) line 1520
nsContainerBox::GetAscent(nsContainerBox * const 0x03649820, nsBoxLayoutState & 
{...}, int & 0) line 591 + 38 bytes
nsBoxFrame::GetAscent(nsBoxFrame * const 0x03649820, nsBoxLayoutState & {...}, 
int & 0) line 1106 + 20 bytes
nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x02d27210, nsIBox * 
0x036496b8, nsBoxLayoutState & {...}, int & 0) line 1520
nsContainerBox::GetAscent(nsContainerBox * const 0x036496b8, nsBoxLayoutState & 
{...}, int & 0) line 591 + 38 bytes
nsBoxFrame::GetAscent(nsBoxFrame * const 0x036496b8, nsBoxLayoutState & {...}, 
int & 0) line 1106 + 20 bytes
nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x02d27210, nsIBox * 
0x02e0c43c, nsBoxLayoutState & {...}, int & 180) line 1520
nsContainerBox::GetAscent(nsContainerBox * const 0x02e0c43c, nsBoxLayoutState & 
{...}, int & 180) line 591 + 38 bytes
nsBoxFrame::GetAscent(nsBoxFrame * const 0x02e0c43c, nsBoxLayoutState & {...}, 
int & 0) line 1106 + 20 bytes
nsSprocketLayout::Layout(nsSprocketLayout * const 0x02d27210, nsIBox * 
0x02e0c43c, nsBoxLayoutState & {...}) line 242
nsContainerBox::DoLayout(nsContainerBox * const 0x02e0c43c, nsBoxLayoutState & 
{...}) line 607 + 34 bytes
nsBoxFrame::DoLayout(nsBoxFrame * const 0x02e0c43c, nsBoxLayoutState & {...}) 
line 1215
nsBox::Layout(nsBox * const 0x02e0c43c, nsBoxLayoutState & {...}) line 1064
nsStackLayout::Layout(nsStackLayout * const 0x02ff5b98, nsIBox * 0x02e0c1a0, 
nsBoxLayoutState & {...}) line 331
nsContainerBox::DoLayout(nsContainerBox * const 0x02e0c1a0, nsBoxLayoutState & 
{...}) line 607 + 34 bytes
nsBoxFrame::DoLayout(nsBoxFrame * const 0x02e0c1a0, nsBoxLayoutState & {...}) 
line 1215
nsBox::Layout(nsBox * const 0x02e0c1a0, nsBoxLayoutState & {...}) line 1064
nsBoxFrame::Reflow(nsBoxFrame * const 0x02e0c168, nsIPresContext * 0x02cc2928, 
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) 
line 1007
nsRootBoxFrame::Reflow(nsRootBoxFrame * const 0x02e0c168, nsIPresContext * 
0x02cc2928, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, 
unsigned int & 0) line 242
nsContainerFrame::ReflowChild(nsIFrame * 0x02e0c168, nsIPresContext * 
0x02cc2928, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 
0, int 0, unsigned int 0, unsigned int & 0) line 790 + 31 bytes
ViewportFrame::Reflow(ViewportFrame * const 0x02e0c12c, nsIPresContext * 
0x02cc2928, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, 
unsigned int & 0) line 577
IncrementalReflow::Dispatch(nsIPresContext * 0x02cc2928, nsHTMLReflowMetrics & 
{...}, const nsSize & {...}, nsIRenderingContext & {...}) line 894
PresShell::ProcessReflowCommands(int 1) line 6461
ReflowEvent::HandleEvent() line 6306
HandlePLEvent(ReflowEvent * 0x03bfba08) line 6320
PL_HandleEvent(PLEvent * 0x03bfba08) line 643 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x01249fb8) line 573 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x005d0136, unsigned int 49447, unsigned int 0, 
long 19177400) line 1308 + 9 bytes
USER32! 77e11b60()
USER32! 77e11cca()
USER32! 77e183f1()
nsAppShellService::Run(nsAppShellService * const 0x0130fcb8) line 472
main1(int 2, char * * 0x00276f30, nsISupports * 0x00000000) line 1508 + 32 bytes
main(int 2, char * * 0x00276f30) line 1869 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e8d326()

Comment 26

[(not reading, please use [e-mail] instead)]

Attachment: update the patch (include sidebar removal)

note, I only updated messenger.xul.  the other file will go away once this bug
is fixed.

Comment 27

[(not reading, please use [e-mail] instead)]

that patch has already rotted a bit, as there is no more sidebar in mail.

Comment 28

[(not reading, please use [e-mail] instead)]

futuring for now, but we'd still love to get this.

putterman also suggests once we don't require a restart, we could move it out 
from prefs and into a menu item.

Comment 29

[[e-mail]]

Comment on attachment 99210 [details] [diff] [review]
update the patch (include sidebar removal)

Now that you've (badly) removed the sidebar I'll rewrite the chome part of the
patch and I'll also address the folder pane flex issues.

>+     setTimeout(EnsureCurrentFolderIsVisible, 0, GetFolderTree());
>     }
>     catch (ex)
>     {
>         dump("Error hiding AccountCentral page -> " + ex + "\n");
>         return;
>     }
>+}
>+
>+function EnsureCurrentFolderIsVisible(folderTree) {
>+  folderTree.treeBoxObject.ensureRowIsVisible(folderTree.currentIndex);
These bits aren't needed any more; fixed by Jan.

>   rv = wwatch->OpenWindow(0, chromeurl.get(), "_blank",
>                  "chrome,dialog=no,all", argsArray,
>                  getter_AddRefs(newWindow));
>-
>+  NS_ENSURE_SUCCESS(rv, rv);
>   return NS_OK;
> }
Is this the same as return rv; ?

Comment 30

[Christian :Biesinger (don't email me, ping me on IRC)]

>>+  NS_ENSURE_SUCCESS(rv, rv);
>>   return NS_OK;
>Is this the same as return rv; ?

No... ENSURE_SUCCESS includes an assertion that NS_SUCCEEDED(rv) is true.

Comment 31

[[e-mail]]

Ah, a debug build thing then :-) Thanks, biesi!

Anyway, back to the patch. Once I get around to making a diff, I can make the
one XUL document open in three layouts: tall folders; folders beside threads
(current alternate) and even folders beside message! but, as you noticed, it
crashes while attempting to dynamically relayout once the window is alredy open :-(

Also there is no "jumping" if you use the columnpickers/load account central.

Comment 32

[[e-mail]]

Attachment: Propertly updated chrome for sidebar removal

Comment 33

[[e-mail]]

Attachment: Fixed JS error in last patch

Comment 34

[scottputterman]

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

Comment 35

[[e-mail]]

Ah, I think there's a problem with the view causing the crash: if I null out the
view first then I don't get a crash, I just get some random JS error instead ...

Comment 36

[[e-mail]]

Attachment: Dyanmic again!

Comment 37

[[e-mail]]

Attachment: polish

Comment 38

[[e-mail]]

Attachment: Corrected diff for msgMail3PaneWindow.js

Comment 39

[David Tenser [:djst]]

Will this patch also fix the annoying (new?) bug that makes the folder pane
resize when resizing the mail window? Or should I file a separate bug report for
that?

(When resizing the mail window horizontally, the folder pane is also resized.
Totally useless and inconsistent, since the Sidebar doesn't resize when resizing
a browser window.)

Comment 40

[[e-mail]]

David: Actually it will, because that's the only way I could stop the folder
pane from resizing when switching between alternate layouts.

But unfortunately Seth is busy implementing spam filters at the moment...

Comment 41

[Samir Gehani]

Mail triage team: need info about whether this will yield any
performance/footprint improvements.

Comment 42

[(not reading, please use [e-mail] instead)]

this isn't a huge win for the end user, so let's keep it futured.

I'd like to see this happen one day, but right now, it's not a lot of gain for
the associated risk.

I'd rather we focus on things that affect the end user more.

but neil, we will get to this one day.

Comment 43

[Samir Gehani]

Mail triage team: nsbeta1-

Comment 44

[[e-mail]]

Attachment: Fixed selection issue

Comment 45

[[e-mail]]

Comment on attachment 115764 [details] [diff] [review]
Fixed selection issue

The XUL/JS has been heavily tested on Win32 but also on Linux; the C++ has only
been tested on Linux.

Comment 46

[[e-mail]]

Attachment: Minor bitrot

Comment 47

[(not reading, please use [e-mail] instead)]

I'm running with this on win32, maybe this can make it for 1.4 alpha.

neil, do notice any performance regressions switching between folders and
account central?

Comment 48

[(not reading, please use [e-mail] instead)]

some other things we might need to worry about:

1) start up performance regressions.

2) those who used mail3PaneWindowVertLayout.xul in previous versions will start
up and find columns in the wrong place (if they moved them) and columns hidden
(or shown), because now they will be using messenger.xul

should we gracefully migrate them by checking the pane config pref, and if set
to use mail3PaneWindowVertLayout.xul, get the appropriate values of localstore
datasource?

Comment 49

[Scott MacGregor]

Attachment: a thunderbird version of Neil's patch + a 3rd 3-pane configuration

With some help from Stephen Walker, I've taken Neil's original patch and
adapted it to thunderbird. I have also added a 3rd 3-pane configuration. 3
vertical panes: folder | thread | message. This is similar to the new Outlook
Express window configuration for those keeping track at home =).

My patch is very similar to Neil's with the following exceptions:

1) Since I am introducing a 3rd config, I'm using a new preference to store the
3-pane configuration. This is to avoid backward compatibility issues where you
set the pref to use the new configuration then go back to and old build and we
barf miserably. 

2) messenger.xul and msgMail3PaneWindow.js differ the most from neil's patch.
For starter's, most of the work when updating the pane configuration is done by
just changing the orient attribute on a several boxes as opposed to re-rooting
boxes. The only box that ever gets re-rooted is the message pane. So a lot of
the re-rooting clean up JS in UpdateMailPaneConfig has been removed.  

3) I was having problems when re-rooting the message pane. The docshell for the
message pane gets destroyed when you pull it out of the dom and then re-insert
it again. I had to reset the docshell on the message pane for messages to
continue to load after dynamically changing the configuration. 

4) the search bar has been moved into another box and not part of the thread
pane. This allows the search bar to stretch out horizontally all the way across
the screen (above both the thread and message pane) when using this new
configuration. 

Neil, can I interest you in double checking my work here and letting me know if
you see anything I did that is really wrong?

Comment 50

[Scott MacGregor]

A couple more comments before I forget. 

1) When moving to the new vertical configuration, I had problems with the
message pane not getting enough horizontal space. The message pane and thread
pane share a parent horizontal box. I want them to equally split the available
horizontal space by default. However, the message pane kept taking just 20
pixels or so. It was way too narrow. I had to hack around this by forcing a
minimum width of 400 pixels on the message pane when we enter the vertical
configuration. I'd like to figure out why the flex isn't working to divide up
the space and remove this hack. I'm open to ideas.

2) For bonus points, it might be nice to remember the width and height of the 3
panes on a per pane configuration basis. I suppose I could persist arbitrary
attributes on each pane and when we unload, save the width and height for the
current config in these arbitrary attributes. When switching to a new config, we
read the arbitrary width/height attributes pertinent to that config. 

Comment 51

[[e-mail]]

Comment on attachment 127418 [details] [diff] [review]
a thunderbird version of Neil's patch + a 3rd 3-pane configuration

OK, since I don't (and hopefully never will) have Thunderbird I can only ever
visually inspect this patch, but here goes:
1. The C++ needs to stop respecting the old pref.
2. This patch has removed the dynamic buttons; that may well be intentional but
that's not within the scope of this bug.
3. I'm not sure you need gMailWindowLoaded; just pass a flag in for if this is
for the initial load or a pref change.
4. You could use switch rather than a bunch of else ifs, also you're comparing
an int to a string constant. Perhaps you should define some consts to make the
code more readable.
5. I think you should clear the previous window before setting the new
messenger window. Originally I did try to move the message pane and it didn't
work but then I didn't know about the docShell issue. So I'll try updating my
patch to move the message pane for comparison.

Comment 52

[Scott MacGregor]

Attachment: updated thunderbird patch

Updated thunderbird patch. Also includes the CPP changes I forgot to include in
the last diff. Most of the changes are to msgMail3PaneWindow.js in this patch. 


There is a problem when moving to the wide layout after viewing a message with
an attachment. There's a crash in layout which I'm looking into, Bug #212364)

Comment 53

[Biju]

Thanks to all for working on Multiple Layout issue.

I dont know whether you guys are doing the same thing I am trying to do.

The way I am going is to split mail3PaneWindowVertLayout.xul  to 
a layout file and few component files.
Where we can have as many STATIC layout files as we like and
and all using same set of component files.


Copy of my current jar is at http://quicktools.mozdev.org/mail.zip
this can be replaced in 
ftp://ftp.mozilla.org/pub/thunderbird/nightly/2003-07-01-trunk/thunderbird-win32.zip


And now I reached following state

In mail.jar

\content\messenger\mail3PaneWindowVertLayout.xul  
     -- in the process of commenting stuff out
\content\messenger\mail3PaneWindowVertLayoutExtra.xul
     -- this I will split again
\content\messenger-views\contents.rdf              
     -- modified to call  mail3PaneWindowVertLayoutExtra.xul    

I wish somebody have look at it.

Why I am looking for STATIC layout file?
Then it will be easy to make new layouts

Comment 54

[Biju]

And here is the screenshots of what I want finally

http://quicktools.mozdev.org/verticalpane4.png
http://quicktools.mozdev.org/verticalpane3.png
http://quicktools.mozdev.org/verticalpane2.png
http://quicktools.mozdev.org/verticalpane1.png

all *,png in one screen 
http://forums.mozillazine.org/viewtopic.php?t=15410

Comment 55

[[e-mail]]

Those tree issues caused other bugs which have since been resolved.

Comment 56

[[e-mail]]

Attachment: Merged in backported thunderbird patch

Comment 57

[Scott MacGregor]

Neil,
A couple things that might block this back port to seamonkey:

1) with the current attachment UI for seamonkey you will crash if you change the
window layout after viewing a message with attachments (Bug #212364)

2) In addition to the thunderbird patch I put in this bug, I also had to remove
the flex on the message pane box ("messagepanebox"). Without this change you get
a lot of jiggling with the thread / message pane splitter especially in the new
wide layout view.

You patch had some code which was updating some menu items when updating the
3-pane layout. Did you add a layout menu item to the view menu (which is
something I have been thinking about doing as well). 

Also, when you back ported the thunderbird version of your path, where there
other changes you made to make it better that I may want to take 'back' to
thunderbird? Boy that's a lot of verbage...

Comment 58

[[e-mail]]

Thanks for looking at this.
1) I stuck in ClearAttachmentList but it occurs to me that I didn't actually
test to see if it fixed it :-)
2) I played around with the widths, heights and flexes to get what appeared to
me to be a stable solution, which you might want to reapply to Thunderbird.
3) I can quickly do you a submenu, but my version of mailWindowOverlay.xul is
quite heavily patched so I wanted to extract it in a separate patch. Plus of
course I'm not exactly sure where this new submenu should go - I've currently
got View - Show/Hide - Layout - Classic / Wide / Vertical

Comment 59

[Scott MacGregor]

About (1). I don't think calling ClearAttachmentsList is going to fix the crash.
Unfortunately the crash is caused by a stale frame reference to the list box
deep in layout and isn't controlled by whether the elements in the list box have
been cleared or not. Just the act of re-rooting a listbox in the document will
always cause the crash, regardless of what we do with the contents of the list box.

Issue #2: I would very much like to try out your solution if you could pick out
what you did! I had lots of issues where the splitter would always move.
Especially in the new wide view when changing between a message with attachments
and one without out. Also, just toggling between the collapsed msg hdr view and
the expanded msg hdr view would cause the splitter to move in the first two
layouts. Removing the flex of the message pane was the only way I could get to
work, but it caused Bug #214838.

Issue #3: I was thinking of a new menu item under view called 'Layout Style' or
something. It would then have a fly out for Classic View, Wide View and Vertical
View and maybe a no preview pane type option for hiding the message pane. Just
my thoughts on how such a menu could look.

I'm happy to give you an sr for this patch if we figure out a solution to the
crash it introduces before landing.

Comment 60

[[e-mail]]

Just tried this patch under 1.4 (win98) and it didn't crash, but I don't yet
know if that's because of or despite the ClearAttachmentList().

Comment 61

[[e-mail]]

Attachment: Proposed patch

Comment 62

[David :Bienvenu]

Comment on attachment 156322 [details] [diff] [review]
Proposed patch

looks good - we might want to port this to tbird.

Comment 63

[[e-mail]]

Fix checked in!