linux., win32. I have, say, 6 messages showing in 3-pane window. I delete them all, then inbox scrolls to some ? random portion of my inbox, and I have to scroll all the way up &back down again for things to show up in the right order again. A 5-second reaction to this problem is "oh, I have no more new mail" which is bad.

Scott or Alec, who wants this one?

I thought at first this might be how I handle selecting the next message after deleting, but that seems to be working, it's selecting the one before the last one deleted. I have seen a similar bug while clicking and I'm wondering if it's a tree widget problem. Sometimes when I click, the tree scrolls to a completely different place and I have to scroll around until I figure out where the selection really is.

This may be fixed by some of the stuff hyatt & I did yesterday, to be checked in today.

ok, it's not fixed, but I think I know what's going on: After all visible content has been deleted, the tree is listed as having no frames...so when reflow happens, it goes to rebuild all the frames. Somehow, GetFirstFrame() must be finding the first content element instead of the last one... it seems like part of what needs to happen is that when you delete n rows, you need to update the scrollbar to reset maxpos=maxpos-n, and set curpos to curpos - n.

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

nominate for dogfood. This is really a bug for the larger problem of frames being recreated when the selection in the tree changes. This problem also causes things like, select the first message, scroll down, select the last message, and the tree refreshes back to the top before you finish selecting the last message.

Putting on PDT+ radar.

ok, I think I've tracked this down to something with nsCSSFrameConstructor::AttributeChanged, but it's hard to debug because mouseovers are causing AttributeChanged to fire.

as usual, hyatt rocks... Turns out OnContentInserted and OnContentRemoved both had problems: OnContentInserted was always setting the top frame to null, and it was blowing away all frames after the current node being removed - which meant that if the content was off-screen, above the tree, it would blow away the whole tree. When the top frame was null, this meant the whole tree would be rebuilt from the top content row in OnContentRemoved, we were again always setting the topframe to null, when in fact we wanted to keep it.. unless of course that was the frame that we're deleting, in which case we just move down one frame. checking in a fix as soon as the tree goes green.

ffffiiixxxeeeeddd!

wooohoooooo!

Using builds 1999120808 on win98, mac and linux, this is not fixed as stated in scenario. I talked with alecf and showed him how it was working on my system. He agrees it was not fixed, he fixed a similiar bug. He will find it and mark it fixed. Reopening.

Argh, so it looks like here's what happened with this bug: - I thought this problem was more general than it was, so I updated the summary from "delete all visible messages, scrolling screws up" to "scrolling screws up when selection changes"... I fixed the general case of the problem, but the specific case of scrolling to the top when you delete all visible messages still exists. Because the description of this bug is for the specific case, the bug has morphed. I'd really like to morph it back, but it's marked PDT+. I nominated the general case of the bug for dogfood though, so here's what I'm going to do: Leave this bug reopened, but take off the [DOGFOOD] and [PDT+]ness of the bug... then I'll log another bug for the more general problem, mark it [DOGFOOD] and [PDT+] and them mark it fixed.

ok, the new bug is #21194

By the way, this particular bug is really tricky. Here's the issue: when we delete the last frame in the toplevel frame, we're calling mTopFrame->GetNextSibling(&mTopFrame) which sets mTopFrame to null... we're also invalidating the cellmap, which means that there's no way for the tree to "catch up" and figure out where it should be. I'm not even sure where it should be though.. I guess we should just bring the last row into view... in 4.x we're smart enough to bring in as many rows as we can to fill up the view. My only thought is to either figure out what the content row before mTopFrame was, and somehow set up the cellmap correctly.

I know how to fix this. In OnContentRemoved, if mTopFrame becomes null, then you need to do the following. (1) Using the old mTopFrame (cache it before you update it), get its content node. (2) Get the next sibling of the top frame. If you have one, it means that there's a frame that needs to be "pulled up". (3) Give the tree row group frame a little content chain and point it at this new content node. (4) Let the tree be marked as dirty (just as it is already). On the reflow, you'll pick up the content node and do the right thing.

we'd like to keep this on PDT+ and mark your new one PDT+ too. Putting back on PDT+ radar.

I'm attaching a proposed patch.

ok, that patch doesn't do anything useful best I can tell. Worth a try.

ah, and I just found out why. The reason is that FindPreviousRowContent fails because the content node has already been removed from the content model... argh! The parent is still set, so I might try fiddling with that.

There are some other issues with this function. We need to talk about this in person I think.

Sure. Just an update - I tried passing the node as the _downward_ hint, which should immediately go to the parent and then pick the last child... this SEEMS like the right thing to do (i.e. the content chain should be set up right, and mTopRow should be null) but it still has the same problem. I'll drop by tomorrow.

ok, I talked to hyatt and I'm attaching a new patch hyatt, mind reviewing? take a look at bug 21343 in case you have any ideas, I don't think that bug is very critical though.

duh, I'm an idiot. Better fix attached that doesn't have this wonderful side effect

Chofmann gives approval IF you have run the smoke tests, and reviewed etc.

heh, thanks for the reminder. I've tested mail, I'll bookmarks/etc

(in addition to regular smoke tests,etc)

done! Fixed!

Using builds 1999121209m12 on win98, and build 1999121308m12 mac and linux, this is fixed as stated in scenario. Verified