Closed Bug 105542 Opened 20 years ago Closed 17 years ago

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

Categories

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

x86
Windows 2000
defect

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.4alpha

People

(Reporter: sspitzer, Assigned: neil)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 15 obsolete files)

34.73 KB, patch
Details | Diff | Splinter Review
30.37 KB, patch
Bienvenu
: review+
Bienvenu
: superreview+
Details | Diff | Splinter Review
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.
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.
Bugzilla Bug 66475 
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
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>
Target Milestone: --- → mozilla1.0.1
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.
Attached patch Basic dynamic support (obsolete) — Splinter Review
This patch allows the mail window to switch the pane config dynamically.
The references to mail3PaneWindowVertLayout.xul still need to be removed.
Keywords: patch, review
Attached patch Final patch (obsolete) — Splinter Review
Attachment #56067 - Attachment is obsolete: true
*** Bug 108828 has been marked as a duplicate of this bug. ***
Blocks: 19147
QA Contact: esther → olgam
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.
Status: NEW → ASSIGNED
Target Milestone: mozilla1.0.1 → mozilla0.9.9
Keywords: nsbeta1+
Priority: -- → P3
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 ?
moving to 1.0.1
Target Milestone: mozilla0.9.9 → mozilla1.0.1
putterman@netscape.com wrote:

> moving to 1.0.1

Why? There's a patch, it's just waiting for the team to have time to review it.
Target Milestone: mozilla1.0.1 → mozilla0.9.9
This patch only updates files in messenger content.
the last patch has js errors.  neil, can you double check your if / else logic 
in UpdateMailPaneConfig()
Attached patch Added missing } (obsolete) — Splinter Review
Attachment #66293 - Attachment is obsolete: true
Attached patch Fix for collapsed folder pane (obsolete) — Splinter Review
This patch fixes problems with starting Mail with a collapsed folder pane, and
fixes problems with the previous patch not persisting some attributes properly.
Attachment #66725 - Attachment is obsolete: true
Keywords: nsbeta1+nsbeta1-
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?
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.
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.
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.
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.


Target Milestone: mozilla0.9.9 → mozilla1.0.1
Blocks: 157217
I remove nsbeta1- keyword - it was for MachV.
Keywords: nsbeta1-
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.
Keywords: nsbeta1
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()
note, I only updated messenger.xul.  the other file will go away once this bug
is fixed.
Attachment #56566 - Attachment is obsolete: true
Attachment #68867 - Attachment is obsolete: true
that patch has already rotted a bit, as there is no more sidebar in mail.
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.
Target Milestone: mozilla1.0.1 → Future
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; ?
>>+  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.
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.
Attachment #99210 - Attachment is obsolete: true
Attached patch Fixed JS error in last patch (obsolete) — Splinter Review
Attachment #99757 - Attachment is obsolete: true
*** Bug 36115 has been marked as a duplicate of this bug. ***
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 ...
Attached patch Dyanmic again! (obsolete) — Splinter Review
Attached patch polish (obsolete) — Splinter Review
Attachment #99824 - Attachment is obsolete: true
Attachment #100537 - Attachment is obsolete: true
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.)
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...
Mail triage team: need info about whether this will yield any
performance/footprint improvements.
Whiteboard: [need info]
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.
Whiteboard: [need info]
Mail triage team: nsbeta1-
Keywords: nsbeta1nsbeta1-
Depends on: 190447
No longer depends on: 190447
Attached patch Fixed selection issue (obsolete) — Splinter Review
Attachment #100727 - Attachment is obsolete: true
Attachment #100775 - Attachment is obsolete: true
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.
Attachment #115764 - Flags: superreview?(sspitzer)
Attachment #115764 - Flags: review?(sspitzer)
Attached patch Minor bitrot (obsolete) — Splinter Review
Attachment #115764 - Attachment is obsolete: true
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?
Assignee: sspitzer → neil
Status: ASSIGNED → NEW
Target Milestone: Future → mozilla1.4alpha
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?
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?
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 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.
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)
Attachment #127418 - Attachment is obsolete: true
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
Those tree issues caused other bugs which have since been resolved.
Attachment #117582 - Attachment is obsolete: true
Attachment #129104 - Flags: superreview?(scott)
Attachment #129104 - Flags: review?(sspitzer)
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...

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
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.
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().
Attachment #115764 - Flags: superreview?(sspitzer)
Attachment #115764 - Flags: review?(sspitzer)
Attached patch Proposed patchSplinter Review
Attachment #129104 - Attachment is obsolete: true
Attachment #156322 - Flags: superreview?(bienvenu)
Attachment #156322 - Flags: review?(bienvenu)
Comment on attachment 156322 [details] [diff] [review]
Proposed patch

looks good - we might want to port this to tbird.
Attachment #156322 - Flags: superreview?(bienvenu)
Attachment #156322 - Flags: superreview+
Attachment #156322 - Flags: review?(bienvenu)
Attachment #156322 - Flags: review+
Product: Browser → Seamonkey
Fix checked in!
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Attachment #129104 - Flags: superreview?(mscott)
Attachment #129104 - Flags: review?(sspitzer)
You need to log in before you can comment on or make changes to this bug.