Closed Bug 73865 Opened 24 years ago Closed 23 years ago

mail/news folder pane should use the new outliner

Categories

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

defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.4

People

(Reporter: cathleennscp, Assigned: hwaara)

References

Details

(Whiteboard: themes:r=andreww,sr=hewitt; content,layout: r=attinasi,sr=hyatt)

Attachments

(38 files)

(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), image/gif
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), image/gif
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), image/gif
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), image/gif
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), image/gif
Details
(deleted), patch
Details | Diff | Splinter Review
need to convert mail/news folder panes to use new outliner
Blocks: 73948
QA Contact: esther → huang
OS: Windows 2000 → All
Hardware: PC → All
Attached patch initial work (deleted) — Splinter Review
r=waterson on ``04/07/01 10:57 add missing addref in nsXULOutlinerBuilder.cpp''
Still to do: SortFolderPane() FillInFolderTooltip() messengerdnd.js rewrite PerformExpandForAllOpenServers() GetSelectedFolder() FolderPaneOnClick() FolderPaneDoubleClick() MsgEmptyTrash() MsgCompactFolder() and maybe more
thanks for working on this, jan. A road block might be dnd. can you drop on a outliner yet?
jan: in your 04/08/01 00:15 patch, why does getIndexOfResource() return |iter.GetRowIndex() + 1|? Shouldn't it return |iter.GetRowIndex()|?
Yes, I already changed that in my tree. But there is another problem. This text is from mail I sent you: I added GetIndexOfResource method to nsXULOutlinerBuilder.cpp NS_IMETHODIMP nsXULOutlinerBuilder::GetIndexOfResource(nsIRDFResource* aResource, PRInt32* aResult) { nsOutlinerRows::iterator iter = mRows.Find(mConflictSet, aResource); if (iter == mRows.Last()) *aResult = -1; else *aResult = iter.GetRowIndex(); return NS_OK; } I need this method for converting folder pane to use RDF outliner. It works well until some container is opened. When this happens, nsOutlinerRows::First() returns second row not first. nsOutlinerRows::First() is used in Find method as start point. But with this hack it works ok: nsOutlinerRows::iterator nsOutlinerRows::First() { iterator result; Subtree* current = &mRoot; // while (current && current->Count()) { if (current) { result.Push(current, 0); current = GetSubtreeFor(current, 0); } result.SetRowIndex(0); return result; } I don't know this code too much and I think you could help me. Thanks.
Attached patch newest patch for RDF outliner (deleted) — Splinter Review
Seth, Ben's been talking about drop feedback...
r=waterson on the last patch. Nice work! (And thanks for wiping my chin on nsOutlinerRows::First().)
Just to let you all know: Seth promised to make a branch where this can be worked on. I've talked to Jan and I will also help out with this once we branch.
I didn't get a chance to start a branch. I'll start one tomorrow, when I get to my linux box. thanks for being patient.
Feel free to land the changes in content/xul/templates/src on the trunk.
I already did that, after ben's super review.
I just filed new bug 75391 (TODO list for folder pane outliner).
Depends on: 75391
Branch tag is FOLDER_OUTLINER_20010410_BRANCH.
Depends on: 75404
Branch build crashes on windows. We should create new one.
I sugges that you re-branch from the _trunk_ (not from your tree, as before) and then re-checkin the stuff that needs to be checked in. It's not _that_ much, anyway. Let me know if I can help you...
New branch name is FOLDER_OUTLINER_20010417_BRANCH
Depends on: 82597
No longer depends on: 82597
Depends on: 82597
Assignee: sspitzer → hwaara
Taking.
New branch name is FOLDER_OUTLINER_20010531_BRANCH. It's ready for testing. Everything should be converted.
Yes, this is great news! Jan merged with the 05-31 tip and converted a lot! But we still have some polishing to do. Would be nice if anyone on the Netscape side would be willing to test (and maybe do some performance measurements on) our branch once it's even more ready for prime-time. Any takers?
Attached patch diff in content (deleted) — Splinter Review
Attached patch diff in layout (deleted) — Splinter Review
Attached patch diff in mailnews (deleted) — Splinter Review
Attached patch diff in themes (deleted) — Splinter Review
Note that the patches are not yet ready for review. This is for testing purposes.
see bug 85088 for a regression our branch will cause.
Leaf: The branch tag is FOLDER_OUTLINER_20010531, and we changed files in themes/, mailnews/ and layout/.
cc'ing lpham, as she'll be doing test builds this week.
don't forget content
Did you tag the whole mozilla tree? Another word, if I pull from FOLDER_OUTLINER_20010531, then it should include all the fixes. Please confirm before I start the test build. Loan
the next two steps are code review and thorough testing. for testing, we need to make sure to run through all the appropriate functional tests. see http://www.mozilla.org/quality/mailnews/tests/index.html running through all those tests will help to lower the risks. for review, we'll need some cycles from a whole bunch of people (since the changes cover content, layout, themes and mailnews.) as for estimating when reviews can be completed, I don't know. personally, this fix isn't top on my list of priorities, since the folder pane currently works and I've got a lot on my plate for 0.9.2 but, we do want this fixed (as all multi-columned trees will eventually be switching over to outliner). following blizzard's guidelines (see http://www.mozillazine.org/articles/article1938.html), this is too risky with not enough reward for 0.9.2 a good next step is to get some branch builds so those that have time, can do some testing.
(This is the second mid-air I got!) The code we have now is very stable, not one known regression except for bug 85088. It works nicely and solid. Would be nice if some NS people could help out with testing the experimental builds once loan has made them? loan: I'm pretty certain it contains all fixes, and yes, I think Jan branched the whole tree (even though we didn't change anything else than the folders I mentioned in my last comment).
it is FOLDER_OUTLINER_20010531_BRANCH
err, sorry for spam. I tagged all tree and it includes all fixes.
sr=hewitt for the CSS changes
Håkan what Seth said is correct, in my opinion. We want this fix, but I think this is too late in 0.9.2 to consider landing this. As for help testing it, I think there will be individuals who will be interested in looking at builds (I'm interested in trying it out), but in the near future Netscape mail QA isn't going to be able to help out with this which was why Seth was suggesting looking at the functional tests on mozilla.
All the stuff will bitrot till 0.9.3! :(
The build failed because mozilla/security/makefile.win didn't have the branch name; therefore makefile.win didn't get pull. I created FOLDER_OUTLINER_20010531_BRANCH for mozilla/security/makefile.win and continue the build. Loan
r = andreww for the folderpane.css changes (which constitutes all the themes changes needed, I'm told)
Whiteboard: themes: r=andreww, sr=hewitt. left to review: mailnews, content, layout
Depends on: 80844
Depends on: 85376
Attached patch complete patch for tip (deleted) — Splinter Review
Changes to popups are already r=pink (bug 82597).
We hope we can get the necessary reviews for 0.9.3 since it won't make it till 0.9.2.
Target Milestone: --- → mozilla0.9.3
My apologies for running this build for performance so late in the game, here's what I found: Methodology here: http://www.mozilla.org/mailnews/win_performance_results.html#method 2001-06-08 Win32 build: Folder Loading Speed 2.78 1.97 2.53 2001-06-11-12 Win32 Outliner build: Folder Loading Speed 2.46 2.06 2.23 2.19 This is on a MS-Windows: 98 Second Edition, 266 MHz Pentium II, 128M RAM machine.
We cut the branch on 05-31, so stephend is so kind that he'll re-run the tests on a 05-31 build. Thanks stephend!
Those numbers will have to do, I can't get a 5-31 build, the earliest I can get is a 6-10 build. ftp://ftp.mozilla.org/pub/mozilla/nightly/
Bienvenu told me to remind him after the NS-branch is released. We're aiming to get this into 0.9.3
Priority: -- → P1
sorry for the delay, and thanks for being patient. on monday (7-16), reviewing / testing and landing this work onto the trunk will be my top priority (unless there is a smoketest blocker.) I'd like to take this bug so that it's officially on my radar. any objections?
I'm cool with that. I'll be around on AIM for questions and general assistance.
Assignee: hwaara → sspitzer
Attached patch latest patch for the tip (deleted) — Splinter Review
One thing I've noticed in the new folder pane is that the gray dotted lines are displayed from the sever icon to all subfolder icons. Is this the desired behaviour? (fyi: This is not the case in the current one)
it's an improvement that's 4xp-win32. hewitt already sr='d for themes so i'd assume he's happy w/ it.
Suresh: Depends on who you ask. If you ask me, one of the authors - sure it's the desired behaviour. I think it looks very cool and it's 4xp.
There are two changes in the latest diff that are not going to work. See bug 88332. messenger.xul and mail3PaneWindowVertLayout.xul both contain an <outliner> tag inside of a <vbox> in the patch. Unless someone can fix 88332 by Monday, you'll have to change those <vbox> tags to <box orient="vertical"> to get correct alignment for now.
I've been using mozilla with this patch for a long time and I don't see any problems with <outliner> inside <vbox>
The orientation of whatever is inside the <vbox> is correct, then? The problem I found was fairly consistent; any <vbox> that contained an <outliner> somehow ended up with horizontal orientation rather than vertical (that would be implied by the <vbox>).
Ok. Disregard my comments. 88332 is invalid. I thought it existed because of a weird overlay in security that expressed orientation of boxes. :)
I'm starting with the last patch attached by Jan. jan / hwaara, while I apply / review / test the mailnews changes, can you find the appropriate people to review the content, layout and xpfe changes? Index: content/xul/content/src/nsXULPopupListener.cpp Index: content/xul/templates/public/nsIXULTemplateBuilder.idl Index: content/xul/templates/src/nsXULOutlinerBuilder.cpp Index: layout/xul/base/public/nsIPopupSetBoxObject.idl Index: layout/xul/base/public/nsIPopupSetFrame.h Index: layout/xul/base/src/nsPopupSetBoxObject.cpp Index: layout/xul/base/src/nsPopupSetFrame.cpp Index: layout/xul/base/src/nsPopupSetFrame.h Index: layout/xul/base/src/outliner/src/nsOutlinerBodyFrame.cpp Index: xpfe/global/resources/content/bindings/popup.xml
I'll review the content/XUL and layout/XUL files, but you should try to get Hyatt to sr the changes there, he's your XUL-dude.
Whiteboard: themes: r=andreww, sr=hewitt. left to review: mailnews, content, layout → themes: r=andreww, sr=hewitt. mailnews: in process by sspitzer. content, layout: in process by attinasi
The layout and content changes look fine to me. But please, what is the constant '21' in the xulPopupListener? + yAdjust += 21; it seems arbitrary, and possibly dependent on something that can change, like resolution, font size, etc. Also, the tabbing seems off (4 spaces instead of 2?) in nsXULOutlinerBuilder.cpp - is that just local convention? r=attinasi
Whiteboard: themes: r=andreww, sr=hewitt. mailnews: in process by sspitzer. content, layout: in process by attinasi → themes: r=andreww, sr=hewitt. left to review: mailnews, content, layout
I think pinkerton reviewed that code earlier, and also noted the weird constant but when he got an explanation, it was rational. Jan?
pinkerton has already reviewed the popup stuff. http://bugzilla.mozilla.org/show_bug.cgi?id=82597 the constant (21) is mentioned there. pink wrote: "we really need to explain the origin of that constant, but that can be left for me to do."
The constant 21 is only moved: - self->LaunchPopup ( self->mMouseClientX, self->mMouseClientY+21); + self->LaunchPopup ( self->mMouseClientX, self->mMouseClientY ) + else if ( popupType == eXULPopupType_tooltip ) { type.AssignWithConversion("tooltip"); + yDelta += 21; + } functionality is the same
jan, I'm getting the following assertion when I try to rename the last folder on my pop account: ###!!! ASSERTION: couldn't find row: 'iter != mRows.Last()', file C:\builds\mozi lla\content\xul\templates\src\nsXULOutlinerBuilder.cpp, line 913
that is apparantly an offset that I added long ago to make tooltips show up below the mouse cursor instead of on top of it.
jan / hwaara: do you see this problem: start up messenger with a lot of accounts (so the folderpane has plenty of folders / newsgroups.) I've got a pop account, imap, local folders, and some news accounts. all the accounts are expanded on startup. after start up, scroll down to the bottom of the folder pane and select something near the bottom. it ends up loading / selecting a folder near the top.
"after start up, scroll down to the bottom of the folder pane and select something near the bottom." select a news server near the bottom. selecting a newsgroup seems to work fine.
Seth, I cannot reproduce your problem. But I found another: sometimes when I click GetMsg, and it receives the messages, the folder that receives the messages doesn't update it's look to be bold and contain the # of new messages in its name. Instead the folder is just plain blank, with no info that there are new messages in it.
Sorry guys, but I can't reproduce described problems with my opt build on linux. Now building debug and will look again.
Hah, a simple restart of the debug build made it; everything works fine now, and I still can't reproduce Seth's problem. So no problem here.
jan / hwaara, try the alternate 3 pane to reproduce the problem I reported on "2001-07-16" with the standard 3 pane, I can't reproduce the problem.
also, load a folder first and then scroll down and click on a server. I'm debugging this problem now.
something is calling ChangeFolderByURI() twice. In ChangeFolderByURI uri = news://sspitzer@news.mcom.com sortType = 22 In ChangeFolderByURI uri = mailbox://nobody@Local%20Folders/Trash sortType = 18 still debugging...
FolderPaneSelectionChange()is being called twice. I think the second one is happening when the outliner is resized. the outliner gets resized when you click on a server because selecting a server loads account central, and loading account central hides the message pane, and hiding the message pane in the vert. 3 pane (unlike messenger.xul) changes the size of the folder pane.
nsOutlinerSelect::Select() is getting called twice when I click on a folder, both from outliner.xuml the first time is on the mousedown handler, the second time on click handler, nsOutlinerSelection::FireOnSelectHandler() line 686 nsOutlinerSelection::Select(nsOutlinerSelection * const 0x0598f670, int 26) line 356 XPTC_InvokeByIndex(nsISupports * 0x0598f670, unsigned int 7, unsigned int 1, nsXPTCVariant * 0x0012d158) line 139 XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode CALL_METHOD) line 1881 + 42 bytes XPC_WN_CallMethod(JSContext * 0x0511ed30, JSObject * 0x045df878, unsigned int 1, long * 0x04611e14, long * 0x0012d38c) line 1252 + 11 bytes js_Invoke(JSContext * 0x0511ed30, unsigned int 1, unsigned int 0) line 807 + 23 bytes js_Interpret(JSContext * 0x0511ed30, long * 0x0012e12c) line 2701 + 15 bytes js_Invoke(JSContext * 0x0511ed30, unsigned int 1, unsigned int 2) line 824 + 13 bytes js_InternalInvoke(JSContext * 0x0511ed30, JSObject * 0x04401660, long 73587768, unsigned int 0, unsigned int 1, long * 0x0012e304, long * 0x0012e254) line 896 + 20 bytes JS_CallFunctionValue(JSContext * 0x0511ed30, JSObject * 0x04401660, long 73587768, unsigned int 1, long * 0x0012e304, long * 0x0012e254) line 3320 + 31 bytes nsJSContext::CallEventHandler(nsJSContext * const 0x0511ec00, void * 0x04401660, void * 0x0462dc38, unsigned int 1, void * 0x0012e304, int * 0x0012e300, int 0) line 942 + 33 bytes nsJSEventListener::HandleEvent(nsJSEventListener * const 0x0630edb0, nsIDOMEvent * 0x0630eb54) line 139 + 57 bytes nsXBLPrototypeHandler::ExecuteHandler(nsXBLPrototypeHandler * const 0x05948030, nsIDOMEventReceiver * 0x05684178, nsIDOMEvent * 0x0630eb54) line 432 DoMouse(nsIAtom * 0x03230e30, nsIXBLPrototypeHandler * 0x05948030, nsIDOMEvent * 0x0630eb54, nsIDOMEventReceiver * 0x05684178) line 102 nsXBLMouseHandler::MouseClick(nsXBLMouseHandler * const 0x05949ca0, nsIDOMEvent * 0x0630eb54) line 117 + 40 bytes nsEventListenerManager::HandleEvent(nsEventListenerManager * const 0x042bbc80, nsIPresContext * 0x0512a160, nsEvent * 0x0012f2dc, nsIDOMEvent * * 0x0012f1e8, nsIDOMEventTarget * 0x05684178, unsigned int 7, nsEventStatus * 0x0012f648) line 1260 + 41 bytes nsXULElement::HandleDOMEvent(nsXULElement * const 0x05684170, nsIPresContext * 0x0512a160, nsEvent * 0x0012f2dc, nsIDOMEvent * * 0x0012f1e8, unsigned int 1, nsEventStatus * 0x0012f648) line 3633 PresShell::HandleEventInternal(nsEvent * 0x0012f2dc, nsIView * 0x00000000, unsigned int 1, nsEventStatus * 0x0012f648) line 5631 + 47 bytes PresShell::HandleEventWithTarget(PresShell * const 0x05136120, nsEvent * 0x0012f2dc, nsIFrame * 0x0446fd8c, nsIContent * 0x05684170, unsigned int 1, nsEventStatus * 0x0012f648) line 5604 + 22 bytes nsEventStateManager::CheckForAndDispatchClick(nsEventStateManager * const 0x051ea800, nsIPresContext * 0x0512a160, nsMouseEvent * 0x0012f754, nsEventStatus * 0x0012f648) line 2456 + 61 bytes nsEventStateManager::PostHandleEvent(nsEventStateManager * const 0x051ea808, nsIPresContext * 0x0512a160, nsEvent * 0x0012f754, nsIFrame * 0x0446fd8c, nsEventStatus * 0x0012f648, nsIView * 0x05949970) line 1541 + 28 bytes PresShell::HandleEventInternal(nsEvent * 0x0012f754, nsIView * 0x05949970, unsigned int 1, nsEventStatus * 0x0012f648) line 5651 + 43 bytes PresShell::HandleEvent(PresShell * const 0x05136124, nsIView * 0x05949970, nsGUIEvent * 0x0012f754, nsEventStatus * 0x0012f648, int 0, int & 1) line 5558 + 25 bytes nsView::HandleEvent(nsView * const 0x05949970, nsGUIEvent * 0x0012f754, unsigned int 8, nsEventStatus * 0x0012f648, int 0, int & 1) line 377 nsView::HandleEvent(nsView * const 0x05131f90, nsGUIEvent * 0x0012f754, unsigned int 28, nsEventStatus * 0x0012f648, int 1, int & 1) line 350 nsViewManager::DispatchEvent(nsViewManager * const 0x05132030, nsGUIEvent * 0x0012f754, nsEventStatus * 0x0012f648) line 2057 HandleEvent(nsGUIEvent * 0x0012f754) line 68 nsWindow::DispatchEvent(nsWindow * const 0x05949834, nsGUIEvent * 0x0012f754, nsEventStatus & nsEventStatus_eIgnore) line 721 + 10 bytes nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012f754) line 742 nsWindow::DispatchMouseEvent(unsigned int 301, nsPoint * 0x00000000) line 4241 + 21 bytes ChildWindow::DispatchMouseEvent(unsigned int 301, nsPoint * 0x00000000) line 4490 nsWindow::ProcessMessage(unsigned int 514, unsigned int 0, long 23068748, long * 0x0012fb94) line 3204 + 24 bytes nsWindow::WindowProc(HWND__ * 0x00120350, unsigned int 514, unsigned int 0, long 23068748) line 989 + 27 bytes USER32! 77e13eb0() USER32! 77e1401a() USER32! 77e192da() nsAppShellService::Run(nsAppShellService * const 0x01120720) line 424 main1(int 3, char * * 0x00485f20, nsISupports * 0x00000000) line 1174 + 32 bytes main(int 3, char * * 0x00485f20) line 1478 + 37 bytes mainCRTStartup() line 338 + 17 bytes KERNEL32! 77e87903()
my instinct is that it is a bug in outliner.xml we are firing select() twice, and that is not good. even in the normal case, we are firing it twice. the only reason we aren't loading the folder twice (in normal cases) is because we've got mailnews js to prevent that.
well, I've got a patch to outliner.xml but it doesn't fix the problem I found. it does prevent us from calling select() twice in the normal case. perhaps we'll also need to suppress select() when it happens during a resize / reflow?
r=hwaara on the supplimental patch. Thanks for finding and fixing this Seth
Attached patch next try (deleted) — Splinter Review
the last patch fixes the problem I was seeing, but I'm nervous about side effects. coupled with my fix for outliner.xml, we're only firing the onselect handler once. jan, can you run your idea by hyatt? I'll attach an updated fix, the complete fix that I'm running in my local tree, and then I'll continue on with my testing and reviewing.
Explanation of the problem: select() method, which fires SelectionChanged() is called from "mousedown" and "click" event handler. Both event handlers call getCellAt() to get index of corresponding row. We're recomputing mInnerBox (used by getCellAt) and mPageCount in Reflow() method. But mousedown is fired before reflow and click after reflow. This way we get two diffrent indexes. The second one is incorrect Solution: Don't change mInnerBox and mPageCount in Reflow() method. Just get it locally hyatt, can you comment what you think ?
I think there is a problem with jan's last patch. It think it is having side effects on the thread pane. see next screen shot.
another problem that I've been having is every so often when I start up, account central doesn't load. it doesn't happen all the time. ###!!! ASSERTION: morkBool_kFalse: '0', file c:\builds\mozilla\db\mork\src\morkC onfig.cpp, line 78 ###!!! ASSERTION: morkBool_kFalse: '0', file c:\builds\mozilla\db\mork\src\morkC onfig.cpp, line 78 ###!!! ASSERTION: morkBool_kFalse: '0', file c:\builds\mozilla\db\mork\src\morkC onfig.cpp, line 78 JavaScript error: line 20: uncaught exception: [Exception... "Component returned failure code: 0x 80004003 (NS_ERROR_INVALID_POINTER) [nsIMsgFolder.getMsgDatabase]" nsresult: "0 x80004003 (NS_ERROR_INVALID_POINTER)" location: "JS frame :: chrome://messenger /content/commandglue.js :: FolderPaneSelectionChange :: line 661" data: no] this might be win32 only. see related bug http://bugzilla.mozilla.org/show_bug.cgi?id=88949 (that's how to reproduce the problem consistently on the trunk)
We were scrolling to negative index in the PositionChange() method and that caused blank area in outliner. This was not visible before because there was another positive ScrollToRow() in Reflow() method.
I'll try out the latest patch from Jan tonight or tomorrow. anyone with a mac want to try out this build: ftp://ftp.mozilla.org/pub/mozilla/nightly/2001-07-16-17-bug_73865 it's the outliner branch build (not the trunk) on the mac. (thanks to leaf for spinning it up.) I've been working on win2k, jan's been doing linux, but we need to give it a go on the mac, too.
Status: NEW → ASSIGNED
Drag and drop on that Mac build highlights the target folder as gold, and the name of the folder is highlighted in black. Known issue? This is with Modern3.
stephend, is your problem in Mac very similar to http://bugzilla.mozilla.org/show_bug.cgi?id=90274 ?
No, it doesn't sound like that bug. When I drag a message over to a folder drop target, the folder name becomes white. (white text on a black background.)
given that nobody was using the outliner folder drop feedback when i wrote it, it's possible there are some coloring bugs with it. those are probably ok and we can clean them up later, no?
Pink: agreed. My comment wasn't of urgency, just an observation. Pretty good, since that's all I can find wrong with the outliner code so far (other than the known issues).
the branch doesn't build on linux for me, dies in security/nss because ../coreconf/Makefile doesn't exist. So, i'm giving up on the branch, and attempting to make a test build using the patch on linux.
leaf, can you apply "fixed problem pointed out on the screenshot" patch after applying main patch ?
jan's last patch appears to have fixed the problem I was seeing. I'll resume my testing and reviewing.
I got side tracked with other reviews (and some 0.9.2 branch bugs). I'll get back to this tomorrow.
as I mentioned a while back, I frequently get the problem where account central doesn't load. related to bugs #81705 and #88949, except I see it very frequently with the outliner patch. I'll investigating it more now. ###!!! ASSERTION: morkBool_kFalse: '0', file c:\builds\mozilla\db\mork\src\morkC onfig.cpp, line 78 ###!!! ASSERTION: morkBool_kFalse: '0', file c:\builds\mozilla\db\mork\src\morkC onfig.cpp, line 78 ###!!! ASSERTION: morkBool_kFalse: '0', file c:\builds\mozilla\db\mork\src\morkC onfig.cpp, line 78 ###!!! ASSERTION: morkBool_kFalse: '0', file c:\builds\mozilla\db\mork\src\morkC onfig.cpp, line 78 ###!!! ASSERTION: morkBool_kFalse: '0', file c:\builds\mozilla\db\mork\src\morkC onfig.cpp, line 78 ###!!! ASSERTION: morkBool_kFalse: '0', file c:\builds\mozilla\db\mork\src\morkC onfig.cpp, line 78 JavaScript error: line 20: uncaught exception: [Exception... "Component returned failure code: 0x 80004003 (NS_ERROR_INVALID_POINTER) [nsIMsgFolder.getMsgDatabase]" nsresult: "0 x80004003 (NS_ERROR_INVALID_POINTER)" location: "JS frame :: chrome://messenger /content/commandglue.js :: FolderPaneSelectionChange :: line 661" data: no] JavaScript error: line 0: uncaught exception: [Exception... "Component returned failure code: 0x8 0070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXULOutlinerBuilder.getResourceAtIndex]" ns result: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: chrome://m essenger/content/msgMail3PaneWindow.js :: GetFolderResource :: line 1007" data: no]
I can't reproduce it on linux, but I suspect that method folderOutliner.outlinerBoxObject.selection.getRangeAt(0, startIndex, endIndex)at 649 line of commandglue.js returns -1 it should not because folderOutliner.outlinerBoxObject.selection.count == 1
I think the general problem is bug #88949, and I've got a fix for it. I'll land it on the trunk and then continue on with my testing and review for this bug. I'll investigate jan's theory about why it might be happening with the outliner in cases where it doesn't happen on the trun.
Depends on: 88949
ok, even with my fix for 88949, I still get a similar problem. I'll attach a screen shot. I no longer get any asserts, but nothing is selected in the folder pane. it might be what jan was talking about. I'll investigate.
When I get into that state, FolderPaneSelectionChange() isn't even being called. still looking...
maybe problem is in loadStartFolder(initialUri)
moving to 0.9.4
Target Milestone: mozilla0.9.3 → mozilla0.9.4
ok, we are bailing out early here: in msgMail3PaneWindow.js, in loadStartFolder(), we return early because var inboxFolder = rootMsgFolder.getFoldersWithFlag(0x1000, 1, outNumFolders); if(!inboxFolder) return; that fails intermittantly. I'm still investigating.
update: 1) I think the intermittant problem with getFoldersWithFlag failing is because we haven't gotten the subfolders into rdf yet. I'll test that theory out. 2) I talked to hyatt, and we need to revisit the solution the previous problem (with the alternate 3 pane). Instead of our last two patches (mine to outliner.xml and jan's to nsOutlinerBodyFrame.cpp), we have to examine outliner.xml. hyatt doesn't think that we should be calling select() twice. he said he'd take (and talk to pink) about why we're doing that.
Interesting, I took look at the tree folder pane. mousedown event is fired before vertical resizing of the folder pane and click is fired after (same behavior as with outliner). But when I click on a server near the bottom of the folder pane (the folder pane gets resized, top folder is moved down) and click event is not fired. Therefore we didn't see this problem before.
I don't think we can fix it only in outliner.xml Both handlers (mousedown and click) calls getCellAt() at the beginning. getCellAt() returns row, column and object. But without patched nsOutlinerBodyFrame.cpp we get two differenct row indexes.
Per Seth, this bug will fix bug 89346.
talking to hyatt, the last two fixes are probably bad. (mine to outliner.xml is, and possibly jan's as well.) the real problem is covered in bug #92366
Depends on: 92366
ok, I've got a fix for the problem (http://bugzilla.mozilla.org/showattachment.cgi?attach_id=43413) I was seeing in my local tree. I'll attach a patch for it. I'll resume my reviewing and testing.
again, sorry for the delay. I've been busy chasing crashers. in my local tree I've got an additional fix: in modern/folderPane.css, I needed to reorder the news rules so that news server would have the right icons. the current patch makes it so all news servers and folders have the same icon, the icon for a news folder. classic/mac/folderPane.css and classic/win/folderPane.css are ok. I'll work on getting an update patch, one that is merged to the trunk (there have been changes underneath us) and one without the two bad patches that were intended to work around #92366 [the select() handler being called twice problem.]
It seems that we have saved some memory: Trunk build after mail window appeared VmSize: 39084 kB VmLck: 0 kB VmRSS: 23808 kB VmData: 19232 kB VmStk: 80 kB VmExe: 56 kB VmLib: 18112 kB Trunk build after scrolling of folder pane VmSize: 39192 kB VmLck: 0 kB VmRSS: 23880 kB VmData: 19340 kB VmStk: 80 kB VmExe: 56 kB VmLib: 18112 kB Trunk with patch after mail window appeared VmSize: 38012 kB VmLck: 0 kB VmRSS: 23624 kB VmData: 18160 kB VmStk: 80 kB VmExe: 56 kB VmLib: 18112 kB Trunk with patch after scrolling VmSize: 38040 kB VmLck: 0 kB VmRSS: 23696 kB VmData: 18188 kB VmStk: 80 kB VmExe: 56 kB VmLib: 18112 kB
ok, the mozilla/mailnews changes look good to me. while we wait for some sr= action from hyatt (and a fix for 92336) I'll continue testing. right now, I'm busy testing the following areas: dnd context menus double click to open new 3 pane check send unsent messages context crap check download flagged / download selected noselect folders unsubscribe to newsgroups empty trash while trash is selected folder compaction I'll attach a complete patch, updated to the tip.
also, while we wait from some hyatt sr=, I'll investigate the other places where folderPane.css was being used, and see if they need to be fixed. http://lxr.mozilla.org/seamonkey/search?string=folderPane.css one known place is the select folder for offline dialog, which I'm working on right now.
whoops, that patch has some addition cruft that is not part of the patch. new patch coming up.
openSaveAttachment.xul is being removed. (see bug #92960) the only real problem is with msgSelectOffline.xul, which is covered in bug #85088 I'm going to work on fix that in my local tree now, so it can land at the same time as this fix.
No longer blocks: 78768
the last patch looks good
adding #89702 because as the thread pane, folder pane and select offline dialog switch to the folder pane, it is really annoying to have double click expand or collapse the thread or folder.
Depends on: 89702
Ok, here are my comments. (1) I don't like the hidden column header stuff in the folder pane outliner. Seems like this should be addressed, especially if you ever have plans of returning to 4.x's three column behavior. (2) I have issues with all the popup code and want to understand it better before allowing API changes to createPopup. This means I can't approve the changes to xpfe (popup.xml) or to any of the popup code in layout and content. Everything else in the last sspitzer patch, though, is sr=hyatt.
I've just commented popup changes in bug 82597.
ok, here's what I'll go do: > (1) I don't like the hidden column header stuff in the folder pane outliner. > Seems like this should be addressed, especially if you ever have plans of > returning to 4.x's three column behavior. agreed. I'll go fix that and fix #54171 along the way. (adding unread / count back to the folder pane.) I got some hints from hyatt how to fix #89702, I'll go work on that too. hyatt thanks for the reviews and comments.
No longer blocks: 54171
talking to hyatt, we've got a new plan. he's doesn't want to take #82597, so I'm going to take those changes out of my tree and come up with a new patch. this means we won't have tooltips in the outliner, but I'm ok with that (see below). note: hewitt is going to work on titletips for the outliner. I'm going to work on #54171 (adding unread and total columns back to the folder pane), and #89702 and finish up #85088. when that's done (assuming I don't find any more problems), we'll just need #92366 (from hyatt) before we can land. I'll be working on my three bugs on monday. once I finish my 3 bugs, I'll probably spin up a new branch for QA to test. It's going to be a monster landing, so we'll need to be cautious.
Blocks: 54171
No longer depends on: 82597
that new patch is the old patch, plus some more stuff, like unread and total columns in the folder pane. (that patch excludes the fix for #82597, so no tooltips.) here's my todo list: 1) can't hide columns in folder pane 2) unread / total in folder pane should be hidden by default 3) bug, don't show "Foldername (Unread Count)" if unread column is not hidden. 4) fix #92366 (hyatt)
> 1) can't hide columns in folder pane fix in hand. on to 2,3.
> 2) unread / total in folder pane should be hidden by default fix in hand. I needed to specify defaults in localStore.rdf. (these defaults will only be for new profiles, not exisiting ones.) new patch on the way.
Attached patch 3) fixed (deleted) — Splinter Review
There are some nits... We should find nicer names for these: http://home.netscape.com/NC-rdf#FolderTreeName http://home.netscape.com/NC-rdf#FolderTreeSimpleName FolderTreeSimpleName is FolderTreeName without unread count
jan's fix for #3 works like a charm. I'll attach some screen shots of the folder pane in the four different modes. about your nit: after this lands, we should rename to this: FolderTreeName -> FolderTreeNameWithUnread FolderTreeSimpleName -> FolderTreeName that's going to be a lot of search and replace, so I'll log a bug to do it later. jan's going to create a new branch so we can land this patch and get builds to QA.
that patch includes two additional fixes. 1)get context menus to work in the folder pane (merge problem) 2)clicking on the name column in the folder pane should not sort the list of folders.
new branch name: FOLDER_PANE_20010807_BRANCH
Blocks: 89346
Whiteboard: themes: r=andreww, sr=hewitt. left to review: mailnews, content, layout → themes:r=andreww,sr=hewitt; content,layout: r=attinasi,sr=hyatt
that latest patch includes fixes for this bug, all the blocker bugs: 80844,85376,89702,92366 and several of the bugs that are blocked by this bug: 54171,85088,89346,93963,94052,93011,78768 the only known problems are: 1) the outliner allows for multiple selection in the folder pane. fixing that is blocked by #94652, which can happen after this lands. 2) no titletips in outliner. see #32157 I'm going to get the folder pane branch up to date, ask lpham to spin new bits, and then get some QA eyeballs on the branch.
Blocks: 78768
There needs to be more space between the icons and the text for each row in the outliner. Compare the screenshot to the current folderpane. This is also a problem with the threadpane, but not with say history (which also uses outliner).
This can be done with padding rules in CSS.
I've fixed the padding problem by adding "padding-right: 2px;" to the rules for the subject column (in the thread pane) and the folder name column (in the folder pane.) I'll attach a new patch.
just tried the speciel build. Here are my comments: - Size, Lines, Unread and Total should be rightaligned - Sometimes when switching between folders I get: Error: uncaught exception: [Exception... "Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsISupports.QueryInterface]" nsresult: "0x80004002 (NS_NOINTERFACE)" location: "JS frame :: chrome://messenger/content/commandglue.js :: ChangeFolderByURI :: line 157" data: no] Source File: Line: 23
This is a problem I found immediately: I found that the Unread/Total columns way too large by default. You can probably make them considerably narrower (fixed="true"?) without cropping the column-name.
Another problem: The history sidebar's outliner is behaving wierd. The "threadlines" are painted over the twisties.
#94653 can't right align text in outliner cell #94813 dotted lines in outliner draw over twisty I'll go work on making the unread and total column smaller by default, and I'll look into the JS error.
In classic, when I have a folder selected, I see a the thread line highlighted for that folder. IE, |- becomes highlighted along with, Inbox. Sheela and I found that renaming a folder leaves a box outline on the folder's name for a short while (but auto-corrects itself).
I've landed. I'll go spin up new bugs on all the known issues. giving back to hwaara, I stole this bug to drive this into the tree.
Assignee: sspitzer → hwaara
Status: ASSIGNED → NEW
fixed.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Blocks: 72481
*** Bug 94408 has been marked as a duplicate of this bug. ***
In order to verifiy this fix, I did the funcational testing on IMAP folders as following: (Verified on all the platforms: 08-15-06-trunk WinNT build & Linux/Mac 08-15-08-trunk builds) Open/View Folders Create Folders Expand/Collapse Folders Copy/Move(Drag & Drop)Msgs to Folders Properties of Folders Rename Folders Subscribe/Unsubscribe Folders I also run some regression testing for IMAP folders & IMAP interop as following: context menus double click to open new 3 pane check msgs from Local folders send unsent messages from the local folder noselect folders empty trash while trash is selected folder compaction check IMAP interop folder panes of AOL, Webmail, UW, Cyrus & MS Exchange. It looks good from above test scenarios for IMAP folders except known [folderpane]bugs. (search these bugs by the status whiteboard) POP/Local folder testing from Sheela. News testing from Stephen. I am marking as verified for this bug. Thanks everyone for helping this bug's verification.
Status: RESOLVED → VERIFIED
great job on verifications! stephen - are there new performance numbers for this new outliner?
The only thing in my test matrix that involves the folder pane is folder loading speed, and that only gained a negligible amount (due to us leaking less). That was a while ago (the 1st time a test build was made), so I'll be re-testing the trunk the day before on Win32 and the new build to see the memory usage (requested by Cathleen, and others). Stay tuned for results Tuesday night or Wednesday morning...
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: