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)
SeaMonkey
MailNews: Message Display
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
Updated•24 years ago
|
QA Contact: esther → huang
OS: Windows 2000 → All
Hardware: PC → All
Comment 1•24 years ago
|
||
Comment 2•24 years ago
|
||
Comment 3•24 years ago
|
||
Comment 4•24 years ago
|
||
r=waterson on ``04/07/01 10:57 add missing addref in nsXULOutlinerBuilder.cpp''
Comment 5•24 years ago
|
||
Comment 6•24 years ago
|
||
Comment 7•24 years ago
|
||
Still to do:
SortFolderPane()
FillInFolderTooltip()
messengerdnd.js rewrite
PerformExpandForAllOpenServers()
GetSelectedFolder()
FolderPaneOnClick()
FolderPaneDoubleClick()
MsgEmptyTrash()
MsgCompactFolder()
and maybe more
Comment 8•24 years ago
|
||
thanks for working on this, jan.
A road block might be dnd.
can you drop on a outliner yet?
Comment 9•24 years ago
|
||
jan: in your 04/08/01 00:15 patch, why does getIndexOfResource() return
|iter.GetRowIndex() + 1|? Shouldn't it return |iter.GetRowIndex()|?
Comment 10•24 years ago
|
||
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.
Comment 11•24 years ago
|
||
Comment 12•24 years ago
|
||
Seth, Ben's been talking about drop feedback...
Comment 13•24 years ago
|
||
r=waterson on the last patch. Nice work! (And thanks for wiping my chin on
nsOutlinerRows::First().)
Assignee | ||
Comment 14•24 years ago
|
||
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.
Comment 15•24 years ago
|
||
sr=ben@netscape.com for the outliner builder patch.
Comment 16•24 years ago
|
||
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.
Comment 17•24 years ago
|
||
Feel free to land the changes in content/xul/templates/src on the trunk.
Comment 18•24 years ago
|
||
I already did that, after ben's super review.
Comment 19•24 years ago
|
||
I just filed new bug 75391 (TODO list for folder pane outliner).
Depends on: 75391
Comment 20•24 years ago
|
||
Branch tag is FOLDER_OUTLINER_20010410_BRANCH.
Comment 21•24 years ago
|
||
Branch build crashes on windows.
We should create new one.
Comment 22•24 years ago
|
||
Assignee | ||
Comment 23•24 years ago
|
||
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...
Comment 24•24 years ago
|
||
New branch name is FOLDER_OUTLINER_20010417_BRANCH
Assignee | ||
Updated•23 years ago
|
Assignee: sspitzer → hwaara
Assignee | ||
Comment 25•23 years ago
|
||
Taking.
Comment 26•23 years ago
|
||
New branch name is FOLDER_OUTLINER_20010531_BRANCH.
It's ready for testing. Everything should be converted.
Assignee | ||
Comment 27•23 years ago
|
||
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?
Comment 28•23 years ago
|
||
Comment 29•23 years ago
|
||
Comment 30•23 years ago
|
||
Comment 31•23 years ago
|
||
Assignee | ||
Comment 32•23 years ago
|
||
Note that the patches are not yet ready for review. This is for testing purposes.
Assignee | ||
Comment 33•23 years ago
|
||
see bug 85088 for a regression our branch will cause.
Assignee | ||
Comment 34•23 years ago
|
||
Leaf: The branch tag is FOLDER_OUTLINER_20010531, and we changed files in
themes/, mailnews/ and layout/.
Comment 35•23 years ago
|
||
cc'ing lpham, as she'll be doing test builds this week.
Comment 36•23 years ago
|
||
don't forget content
Comment 37•23 years ago
|
||
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
Comment 38•23 years ago
|
||
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.
Assignee | ||
Comment 39•23 years ago
|
||
(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).
Comment 40•23 years ago
|
||
it is FOLDER_OUTLINER_20010531_BRANCH
Comment 41•23 years ago
|
||
err, sorry for spam.
I tagged all tree and it includes all fixes.
Comment 42•23 years ago
|
||
sr=hewitt for the CSS changes
Comment 43•23 years ago
|
||
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.
Assignee | ||
Comment 44•23 years ago
|
||
All the stuff will bitrot till 0.9.3! :(
Comment 45•23 years ago
|
||
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
Comment 46•23 years ago
|
||
The build completed and it is on
ftp://ftp.mozilla.org/pub/mozilla/nightly/2001-06-11-06-bug_73865/
Loan
Comment 47•23 years ago
|
||
r = andreww for the folderpane.css changes (which constitutes all the
themes changes needed, I'm told)
Assignee | ||
Updated•23 years ago
|
Whiteboard: themes: r=andreww, sr=hewitt. left to review: mailnews, content, layout
Comment 48•23 years ago
|
||
Comment 49•23 years ago
|
||
Changes to popups are already r=pink (bug 82597).
Assignee | ||
Comment 50•23 years ago
|
||
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.
Assignee | ||
Comment 52•23 years ago
|
||
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/
Assignee | ||
Comment 54•23 years ago
|
||
Bienvenu told me to remind him after the NS-branch is released. We're aiming to
get this into 0.9.3
Priority: -- → P1
Comment 55•23 years ago
|
||
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?
Assignee | ||
Comment 56•23 years ago
|
||
I'm cool with that.
I'll be around on AIM for questions and general assistance.
Assignee: hwaara → sspitzer
Comment 57•23 years ago
|
||
Comment 58•23 years ago
|
||
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)
Comment 59•23 years ago
|
||
it's an improvement that's 4xp-win32. hewitt already sr='d for themes so i'd
assume he's happy w/ it.
Assignee | ||
Comment 60•23 years ago
|
||
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.
Comment 61•23 years ago
|
||
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.
Comment 62•23 years ago
|
||
I've been using mozilla with this patch for a long time and I don't see any
problems with <outliner> inside <vbox>
Comment 63•23 years ago
|
||
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>).
Comment 64•23 years ago
|
||
Ok. Disregard my comments. 88332 is invalid. I thought it existed because of
a weird overlay in security that expressed orientation of boxes. :)
Comment 65•23 years ago
|
||
Comment 66•23 years ago
|
||
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
Comment 67•23 years ago
|
||
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.
Assignee | ||
Updated•23 years ago
|
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
Comment 68•23 years ago
|
||
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
Assignee | ||
Comment 69•23 years ago
|
||
I think pinkerton reviewed that code earlier, and also noted the weird constant
but when he got an explanation, it was rational. Jan?
Comment 70•23 years ago
|
||
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."
Comment 71•23 years ago
|
||
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
Comment 72•23 years ago
|
||
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
Comment 73•23 years ago
|
||
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.
Comment 74•23 years ago
|
||
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.
Comment 75•23 years ago
|
||
"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.
Assignee | ||
Comment 76•23 years ago
|
||
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.
Comment 77•23 years ago
|
||
Sorry guys, but I can't reproduce described problems with my opt build on linux.
Now building debug and will look again.
Assignee | ||
Comment 78•23 years ago
|
||
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.
Comment 79•23 years ago
|
||
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.
Comment 80•23 years ago
|
||
also, load a folder first and then scroll down and click on a server.
I'm debugging this problem now.
Comment 81•23 years ago
|
||
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...
Comment 82•23 years ago
|
||
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.
Comment 83•23 years ago
|
||
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()
Comment 84•23 years ago
|
||
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.
Comment 85•23 years ago
|
||
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?
Comment 86•23 years ago
|
||
Assignee | ||
Comment 87•23 years ago
|
||
r=hwaara on the supplimental patch. Thanks for finding and fixing this Seth
Comment 88•23 years ago
|
||
Comment 89•23 years ago
|
||
Comment 90•23 years ago
|
||
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.
Comment 91•23 years ago
|
||
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 ?
Comment 92•23 years ago
|
||
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.
Comment 93•23 years ago
|
||
Comment 94•23 years ago
|
||
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)
Comment 95•23 years ago
|
||
Comment 96•23 years ago
|
||
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.
Comment 97•23 years ago
|
||
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.
Comment 99•23 years ago
|
||
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.)
Comment 101•23 years ago
|
||
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).
Comment 103•23 years ago
|
||
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.
Comment 104•23 years ago
|
||
leaf, can you apply "fixed problem pointed out on the screenshot" patch
after applying main patch ?
Comment 105•23 years ago
|
||
jan's last patch appears to have fixed the problem I was seeing.
I'll resume my testing and reviewing.
Blocks: 67452
Comment 106•23 years ago
|
||
I got side tracked with other reviews (and some 0.9.2 branch bugs).
I'll get back to this tomorrow.
Comment 107•23 years ago
|
||
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]
Comment 108•23 years ago
|
||
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
Comment 109•23 years ago
|
||
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
Comment 110•23 years ago
|
||
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.
Comment 111•23 years ago
|
||
Comment 112•23 years ago
|
||
When I get into that state, FolderPaneSelectionChange() isn't even being called.
still looking...
Comment 113•23 years ago
|
||
maybe problem is in loadStartFolder(initialUri)
Comment 115•23 years ago
|
||
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.
Comment 116•23 years ago
|
||
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.
Comment 117•23 years ago
|
||
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.
Comment 118•23 years ago
|
||
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.
Comment 119•23 years ago
|
||
Per Seth, this bug will fix bug 89346.
Comment 120•23 years ago
|
||
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
Comment 121•23 years ago
|
||
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.
Comment 122•23 years ago
|
||
Comment 123•23 years ago
|
||
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.]
Comment 124•23 years ago
|
||
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
Comment 125•23 years ago
|
||
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.
Comment 126•23 years ago
|
||
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.
Comment 127•23 years ago
|
||
Comment 128•23 years ago
|
||
whoops, that patch has some addition cruft that is not part of the patch. new
patch coming up.
Comment 129•23 years ago
|
||
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
Comment 130•23 years ago
|
||
Comment 131•23 years ago
|
||
the last patch looks good
Comment 132•23 years ago
|
||
Comment 133•23 years ago
|
||
Comment 134•23 years ago
|
||
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
Comment 135•23 years ago
|
||
Comment 136•23 years ago
|
||
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.
Comment 137•23 years ago
|
||
I've just commented popup changes in bug 82597.
Comment 138•23 years ago
|
||
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
Comment 139•23 years ago
|
||
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
Comment 140•23 years ago
|
||
Comment 141•23 years ago
|
||
Comment 142•23 years ago
|
||
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)
Comment 143•23 years ago
|
||
> 1) can't hide columns in folder pane
fix in hand.
on to 2,3.
Comment 144•23 years ago
|
||
> 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.
Comment 145•23 years ago
|
||
Comment 146•23 years ago
|
||
Comment 147•23 years ago
|
||
Comment 148•23 years ago
|
||
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
Comment 149•23 years ago
|
||
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.
Comment 150•23 years ago
|
||
Comment 151•23 years ago
|
||
Comment 152•23 years ago
|
||
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.
Comment 153•23 years ago
|
||
new branch name: FOLDER_PANE_20010807_BRANCH
Comment 154•23 years ago
|
||
Comment 155•23 years ago
|
||
Updated•23 years ago
|
Whiteboard: themes: r=andreww, sr=hewitt. left to review: mailnews, content, layout → themes:r=andreww,sr=hewitt; content,layout: r=attinasi,sr=hyatt
Comment 156•23 years ago
|
||
Comment 157•23 years ago
|
||
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
Comment 158•23 years ago
|
||
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).
Comment 159•23 years ago
|
||
This can be done with padding rules in CSS.
Comment 160•23 years ago
|
||
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.
Comment 161•23 years ago
|
||
Comment 162•23 years ago
|
||
Comment 163•23 years ago
|
||
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
Assignee | ||
Comment 164•23 years ago
|
||
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.
Assignee | ||
Comment 165•23 years ago
|
||
Another problem:
The history sidebar's outliner is behaving wierd. The "threadlines" are painted
over the twisties.
Comment 166•23 years ago
|
||
#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).
Comment 168•23 years ago
|
||
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
Comment 170•23 years ago
|
||
*** Bug 94408 has been marked as a duplicate of this bug. ***
Comment 171•23 years ago
|
||
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
Comment 172•23 years ago
|
||
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...
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•