Closed Bug 105542 Opened 23 years ago Closed 20 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)

(deleted), patch
Details | Diff | Splinter Review
(deleted), 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) (deleted) — 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) (deleted) — 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
Attached patch Original patch updated for bitrot (obsolete) (deleted) — Splinter Review
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) (deleted) — Splinter Review
Attachment #66293 - Attachment is obsolete: true
Attached patch Fix for collapsed folder pane (obsolete) (deleted) — 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()
Attached patch update the patch (include sidebar removal) (obsolete) (deleted) — Splinter Review
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.
Attached patch Propertly updated chrome for sidebar removal (obsolete) (deleted) — Splinter Review
Attachment #99210 - Attachment is obsolete: true
Attached patch Fixed JS error in last patch (obsolete) (deleted) — 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) (deleted) — Splinter Review
Attached patch polish (obsolete) (deleted) — Splinter Review
Attachment #99824 - Attachment is obsolete: true
Attachment #100537 - Attachment is obsolete: true
Attached patch Corrected diff for msgMail3PaneWindow.js (obsolete) (deleted) — Splinter Review
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) (deleted) — 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) (deleted) — 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.
Attached patch updated thunderbird patch (deleted) — Splinter Review
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.
Attached patch Merged in backported thunderbird patch (obsolete) (deleted) — Splinter Review
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 patch (deleted) — Splinter 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: 20 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.

Attachment

General

Created:
Updated:
Size: