Closed Bug 57139 Opened 24 years ago Closed 24 years ago

Subject list scrolls undesirably while reading messages

Categories

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

Product:
SeaMonkey
Component:
MailNews: Message Display
Platform:
x86
Windows NT
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: jar, Assigned: hyatt)

References

Details

(Whiteboard: [rtm++])

Attachments

(4 files)

Part 1 of 3
10.69 KB, patch
Details | Diff | Splinter Review
Part 2 of 3
4.66 KB, patch
Details | Diff | Splinter Review
Part 3 of 3
1.39 KB, patch
Details | Diff | Splinter Review
Comprehensive single patch for the trunk.
28.89 KB, patch
Details | Diff | Splinter Review 
a) Go into non-threaded view for inbox
b) Select reverse chronological order (re: most recent messages at top of
subject list)
c) Scroll down one or two pages of subjects from the top of your inbox
d) Select a message (as though reading messages sequentially)
e) Have a message arrive in your inbox

Expected result: No change in scrolling position of messsage subject list

Undesired result: Message list scrolls to top of list so that you can see the
new subject header.  This is nice 'cause you get to see the new mail... but evil
because you loose your position in the subject list and must scroll your way
back down to continue your reading :-(

I thought this was filed... but now I'm told it is was not.

Side possibility: It *might* be that the thumb in the elevator bar for the
subject pane remains at the "lower level," even though the actual visible list
of subjects "scrolls" to the top of the list.
Since mail folks were pretty alarmed about this bug (relative to other
usabiliity bugs they have addressed), I'm nominating this for rtm.

Note that unless this had a Really Small fix, I don't believe there is much
justification for repairing this at this late date.
Adding Jar's RTM keyword.  Jar, are you also experiencing form submit problems ?-)

Scott, do we agree the behavior is incorrect?  What does 4.x do?  What's your
estimate of the change required?
Assignee: selmer → putterman
Keywords: rtm
Whiteboard: [need info]
reassigning to hyatt.

I don't know the answer to this.  I'm pretty sure that we're just telling RDF we
have a new item and when it gets put in the top, it's scrolling there.  The
opposite doesn't happen when you are adding to the bottom.
Assignee: putterman → hyatt
QA Contact: esther → laurel
Clearing need info, copying trudelle. Peter, can you evaluate for RTM?
Whiteboard: [need info]
This doesn't sound like it fits the profile for RTM, since the inconvenience is
so slight, and presumably affects very few users. However, on Win98 this scrolls
seemingly without updating the srcollbar, so they get out of whack, and
subsequent attempts to scroll up just smear the new message.  Possibly worth
some investigation for RTM, so setting rtm need info.  Still a P3 though.
Whiteboard: [rtm need info]
Target Milestone: --- → M18
Just for the record... the inconvenience is very great, assuming you have a 
large inbox, and have automated checking for email (every n minutes).  Simply 
put, you can be in the middle of your inbox reading a message, and the entire 
message list will "scroll away."  You have *no* way to get back where you were 
reading messages prior to the auto-scroll.  In nav 4.x, you could just re-sort 
the list, and the "current" message would appear in the visible area.  This is 
not true in Netscape 6. There is, IMO, no way to just continue reading your 
email.  

The closest you get is to turn off automatic checking of email... but I consider 
that a very nice feature, and a shame to loose because of this bug :-/.
This is a bug in the RDF sort service.  The sort service is actually removing
and re-inserting the outermost tree children when new messages come in.  The
sort service should only ever do this when the sort being imposed on the tree
changes.

It cannot do this just because new messages have come in.  Maybe this was a
perceived performance optimization to avoid multiple notifications, but it
should be undone if so.  Even if I enhanced the treechildren so that it could
save and restore its scroll position, new messages are being inserted, so I
wouldn't be able to restore correctly anyway if yanked.

We should patch the sort service to behave more nicely.  I'm smart about not
wasting time if the insertion happens offscreen, so I don't need the sort
service to be yanking my outermost row group (treechildren) out of the tree and
putting it back in unless a resort has actually occurred.

Reassigning to waterson.
Assignee: hyatt → waterson
Note that if this is fixed, then bug #57483 will become more important, since
instead of scrolling up to the top, you'll end up staying where you are, but the
scrollbar will end up out of sync with the tree.

I'm going to connect these two bugs with a dependency.
Depends on: 57483
hyatt says I'm off the hook.
Assignee: waterson → hyatt
I have a one-line patch that fixes this problem... however, note that 57483 
then becomes the issue.  Given that with the jumping you're also out of sync, 
fixing this bug alone does put you in a better situation, i.e., you're still 
out of sync, but at least you didn't jump.
Status: NEW → ASSIGNED
Attached patch Part 1 of 3 — — Splinter Review 

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Part 2 of 3
        — 
          — Splinter Review
      

    


  

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Part 3 of 3
        — 
          — Splinter Review
      

    


  

    
  
Blocks: 57280

    
    

    
  
Jeepers... what happened to the one-line-fix that would get me most of the way
back to Kansas??
These three patches don't look especially safe at this point in the RTM cycle
:-( :-(.
Any chance you'll fess up with the one liner???

    
    

    
  
The one-liner didn't work. :(  It was much more involved than I thought.
Basically the tree widget gets totally screwed whenever content is removed,
inserted, or appended above you offscreen.

It also gets screwed on async appends (which is what FTP was doing).  This patch
fixes all of those problems, but yeah, it's bigger.


    
    

    
  
The code we currently have in these areas is *known* to be very *unsafe*. Rather
than just the usual r=/a=, I suggest we get a team of software proctologists to
examine every token of these patches, land them on the trunk, and have QA run
every test known to man.  We *need* these problems fixed.  cc bryner, alecf for
starters.  Who else?

    
    

    
  
(this is Jar with his PDT hat off)
hmmm... just for the record... other folks were not especially sympathetic to
this bug of mine.  It sounds like you now understand the issue well enough that
you *might* be able to illuminate even more evil scenarios, and gather some
support...
Please do so if you can (i.e., describe evil side effects in all their glory).

As Peter noted (and I paraphrase here) "the patch is big." I'm sure the PDT
would need a very crisp understanding of the problem, and a concise explanation
of why the fix works, and won't break any other working parts of the universe,
before even contemplating such a landing.

Bottom line: In addition to the patch, a double plus would require a strong
benefit statement, and a compelling exposition that risk was closer to zarro
than Peter's (and my) observation would suggest.

Thanks,
Jim (who personally wishes this bug gets fixed... but won't hold breath or N6)

    
    

    
  
Jim, this patch does have more broad benefits than fixing the issue you reported
here.  It looks like hyatt's patches address all sorts of tree manipulation
problems.  This goes beyond this bug as it would also affect the FTP and file
system views in Mozilla.  Currently, those views are broken in a way which
amounts to data loss (bug 57280, which depends on this).

Having a tree view which actually works (the current tree widget simply does not
work in many mainstream cases) will significantly increase the appearance of
Mozilla as a quality product.  Shipping NS6 with the current tree widget will
give the appearance of a very shoddy product, as the users will not be able to
browse FTP sites or their own local disks, and as you note there are also UI
nuisances.

Hope that helps.

    
    

    
  
were these made against the branch or the trunk? they're not patching cleanly
against the branch.

    
    

    
  
I had problems on both trees, partly due to embedded ctrl-Ms but one of the
files seems to have changed enough that I don't think I can do a confident job
hand editing like I had to with the rest. David, could you attach
nsCSSFrameConstructor.cpp verbatim or mail to me? Branch would be preferrable,
but I will take whatever you have.


    
    

    
  
adding self to cc list. If we could get clean patches for the branch, I'd be
happy to try them out.

    
    

    
  
Patching nsCSSFrameConstructor.cpp on the branch to account for these changes is
nontrivial.  I'm not going to waste time doing it until I know these bugs will
actually be approved on the branch.  You should just test on the trunk.  if they
approve the bugs, then the tree widget on the trunk will become exactly
equivalent to the one on the branch when they are landed.


    
  
Blocks: 56979

    
  
No longer depends on: 57483

    
  
Blocks: 57483

    
    

    
  
I hate to say this, but I still see problems in the thread pane on the trunk.
They are mostly paint type problems (e.g., the wrong message looks selected, or
selection looks multiple when it's only single) and the problem where the thread
pane stops painting, I think caused by expanding a large thread at the bottom of
the thread pane or at the top.

    
    

    
  
Was this checked into the trunk?


    
    

    
  
hmm, maybe not. I thought they were, for some reason.

    
    

    
  
No, this isn't checked in yet.  I need an r= and an a= please.


    
    

    
  
I see the stop-painting problem and the message-looks-selected problem on the
branch all the time... most of the time on linux, but occasionally on windows too

    
    

    
  


  

    
    

    
  
This patch seems like a huge improvement for the mail/news thread pane. Things
like multiple deletes are a lot smoother, expanding a large thread is much
faster (is it possible that these changes would speed that up?), I haven't seen
the selection get weird yet, or had things jump around like crazy. And, when I
create a new local folder, it shows up w/o having to collapse and expand the
server. So far, so good.

    
    

    
  
Yes, open/close will be faster with this patch, because the parent is no longer 
reframed (only the kids).


    
    

    
  
I see something like a 3x speedup on expanding a thread of 30 messages - it's
more or less instantaneous, whereas before, there was a big pause. waterson or
alec, can we get an r=? I'd review it, but it wouldn't be meaningful since I
don't know the code :-)

    
    

    
  
hyatt and I were looking at one failure mode last night, where biff arrival
of messages into a threaded view of the threadPane could confuse the tree.
However, I do agree that most operations are more stable, and even when I 
could get it "confused", it was relatively easier to recover (and I haven't 
made it crash at all). 

    
    

    
  
Ok, I've tried to look this over and I just don't know enough about the tree
anymore...(it's changed alot since I poked at it) I wish I could confidently say
r=alecf, but I just can't :(

    
    

    
  
yes, I've seen that too - what happens is after deleting the newly arrived mail,
the selection gets a bit confused (the deleted line doesn't go away, and the
selection looks multiple). It might have to do with the newly arrived mail
getting added to the end of the thread pane, but I'm not positive. That happened
without the patch too.

    
    

    
  
bienvenu, did you see this in unthreaded mode or threaded mode? 

    
    

    
  
always threaded, *but* the newly arrived message was not part of another thread,
i.e., it was its own thread, as was the message that was selected after the
newly arrived message was deleted. This makes me suspect that it might happen in
non-threaded mode too, but most of my folders are threaded.

    
    

    
  
Do you sort by date, and if so, do your new messages come in at the top or
bottom?  I want to start running with your setup so I can debug these glitches.


    
    

    
  
yes, sorted by date, and new msgs come in at the bottom (or sometimes right
before the bottom). I'll try to see if I notice a pattern as to when it happens.

    
    

    
  
Dave, I've noticed that this bug happens coincidentally with the message headers
not showing up bug, and that when I read a message, the unread count in the
folder pane is not updated either - it's as if all layout changes are blocked
until I move the splitter. Don't know if this helps...

    
    

    
  
to be clear, here's what happens:

1. I open a folder with unread messages in it.
2. Scroll to the bottom
3. Select the last message (which is unread)

The message header area does not appear. The message does not noticeably get
marked read, nor does the folder unread count in the folder pane go down. I can
mark other messages read or unread by clicking on the green diamond, but the
state change is not reflected in the UI until I move the folder/thread pane
splitter. Very odd.

    
    

    
  
Got some nice clear steps to reproduce?


    
    

    
  
D'oh.  You just answered me.


    
    

    
  
jrgm, could this be one of those "magic height" kinda things, i.e., if you set 
the splitter to certain heights, the tree gets messed up?


    
    

    
  
it seems to happen when we had to download new headers for an imap folder. I'm
not sure why that should matter since we download the headers before we tell rdf
about any of the headers. I'll look into it a little more.


    
    

    
  
Possibly. I just tried bienvenu's steps, and didn't get this behaviour (opt.
win2k with your patch). I'll try varying the size of the threadPane.

One question though: when you say "The message header area" are you referring 
to the "Subject:, From:, Date:" toolbar? 

    
    

    
  
Yes, that's what I'm referring to. But I now think those not appearing is
orthogonal to the other bugs - the headers can disappear when the other
layout/repaint bugs don't happen. I haven't seen it the other way around (the
headers appear but the other bugs happen) but that could be because the headers
almost always disappear when I open a folder. Hope that makes sense!

    
    

    
  
I saw what you described (where the message headers didn't come in, and the 
Read count was not changed in the UI, plus when I scrolled I began to get 
blank lines), but haven't quite been able to do it again. I think that this
must be on a newly visited folder (no message selected, and the last message
at the bottom must be partially covered by the splitter. 

    
    

    
  
looks good to me, although it would be nice to get this problem bienvenu is
seeing fixed.

r=bryner


    
    

    
  
But that problem is not caused by this patch, is it?

    
    

    
  
No, that bug existed before this patch. I think bryner is just saying that it
would be good to nail all the problems we know about at once. I've been running
with this patch for a few days and it seems great. Aim is working fine as well
(I heard there were some tree problems with AIM as well)

    
    

    
  
Yeah, that's what I figured, but I want to ensure that everything is explicit
and clear for PDT triage.  We may only have one more chance at triage, so I want
to get ++ on Monday, not 'need info'.  I don't think that adding to this patch
to address more cases would be a good idea at this point, because it would mean
less testing time, but if we get a second chance, fixing that may be worthwhile.

    
    

    
  
Here's another clue - when I'm in this mode, if I cause a repaint to happen (by
bringing another window forward and then bringing the mail window back to the
top), the folder pane repaints correctly, and the thread pane mostly repaints
correctly, so it looks like reflow is happening, but lines in the tree aren't
getting invalidated. I say "mostly repaints correctly" because I did get one of
the lines painted blank.

    
    

    
  
a=ben@netscape.com

    
    

    
  
I inflicted the a= process on ben.  Thanks for taking the time to listen to the 
brain dump, ben!

Whiteboard: [rtm need info] → [rtm+]

    
    

    
  
that a=ben@netscape.com was from me, but posted from hyatt's machine.

    
  
Whiteboard: [rtm+] → [rtm+] FIXED ON TRUNK

    
    

    
  
PDT: This fixes about 6 or 7 filed rtm bugs on the threadpane, and it makes
everything feel MUCH better when dealing with trees.

Although the patch is large, the effects are a definate winner for RTM.


    
    

    
  
This bug is in candidate limbo.  We will reconsider this fix once we have a 
candidate in hand, but we can't take this fix before then.  Please check into 
the trunk ASAP.

    
    

    
  
Marking RTM double plus.
Per discussion at PDT, we would like this checked into the BRANCH this evening
(Tuesday eve, 10/30).  We have a tag of the build done earlier in the evening,
and would like to have tomorrow morning's build include this checkin.
As discussed, this is something of a "carpool" for a larger than average limbo bug.
Thanks,
Jim (for PDT, with fingers crossed... shooting the limbo moon) Roskind
Whiteboard: [rtm+] FIXED ON TRUNK → [rtm++] FIXED ON TRUNK

    
    

    
  
fixed

Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Whiteboard: [rtm++] FIXED ON TRUNK → [rtm++]

    
    

    
  
jar - do you see this bug any longer with the limbo builds?

    
    

    
  
Looks OK to me using nov02 (limbo2) mn6 commercial branch build with win98,
linux rh6.0 and mac OS 9.0


Keywords: vtrunk

    
    

    
  
This bug is fixed as far as I can see.  I no longer see the strange scrolling
behaviour.

There is one tiny artifact left, but it might be similar to what happens in 4.x.
 If I'm reading email, and the scroll bar in the subject pane is at the very top
(i.e., I can see the subject line for the most recent message), but I'm reading
(for example) the fourth message down (and hence it is highlighted), and a new
message arrives.... then the message pane scrolls when a new message arrives.
IMO, the arrival of a new message should cause the creation of a new subject
line *above* the top of the header window, and out of view. (Recall that my
messages are in reverse chronoligical order, so new messages go to the TOP of
the list).  Note that if I can't see the first message, then arrival of new
messages *don't* cause scrolling of the subject pane.

Oh well... subtle point... but the basic bug I hit is Very Fixed now.
Thanks,
Jim

    
    

    
  
The original problem is OK using commercial trunk builds, dec6, linux rh6.0,
Win98 and Mac OS 9.0.

I'll log the remaining issue jar mentions (I see the problem) as a separate bug.
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: