Closed Bug 77506 Opened 23 years ago Closed 23 years ago

Sometimes selecting subject line does not display message

Categories

(SeaMonkey :: MailNews: Message Display, defect)

Product:
SeaMonkey
Component:
MailNews: Message Display
Platform:
x86
Windows NT
Type:
defect
Priority:
Not set
Severity:
major

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.1

People

(Reporter: Peter, Assigned: sspitzer)

References

Details

(Whiteboard: Have Fix - would like for 0.9)

Attachments

(2 files)

the fix
2.14 KB, patch
Details | Diff | Splinter Review
fix, please review
5.06 KB, patch
Details | Diff | Splinter Review 
Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:0.8.1+) Gecko/20010424, imap server

Sometimes selecting subject line does not display message.

I don't know exactly when this occurs, but it could be after I either quickly
select another messager, drag a message to a local folder and then select
another imap msg, or delete (mark as deleted) a msg and select another msg.

The only workaround is to select a local folder and then reselct an imap message :(
Keywords: mozilla0.9.1, nsCatFood
Keywords: 4xp
confirming, I've seen this too.

bienvenu, have you seen it?
Status: UNCONFIRMED → NEW
Ever confirmed: true
yes, I've seen it. The workaround is to select *any* other folder, and come back
to the original folder. This means the problem is that for some reason we
haven't cleared the state that says we're loading a message (or the state that
sez we're deleting a message, can't remember which, I'll go look).
so here's the deal - if you accidentally click on the thread/message pane
splitter in any place other than the grippy in the middle, you'll get into this
state. To fix it, just click on the grippy twice. In the js code that suppresses
message display if you collapse the message pane, we're not paying attention to
whether the grippy was clicked, or just the splitter. Seth's looking for a fix.
testing a fix now, to move our onclick handler from the splitter to the grippy.

thanks for doing the debugging work, dave.
Unless I'm reading this wrong, I think this is the same as bug 76566
QA Contact: esther → laurel
*** Bug 76566 has been marked as a duplicate of this bug. ***
racham, ssu, or varada:  can I get a review?
Status: NEW → ASSIGNED
r=racham
thanks bhuvan.

this has sr=bienvenu

I'll try to land this today.

if we haven't branched, I'll see if I can get it into 0.9

otherwise, it will be there for 0.9.1
Target Milestone: --- → mozilla0.9.1
wait, that's part of the problem, but that's not the whole problem.

that fix just makes it so this badness will only bite us when you actually click
on the grippy, instead of anywhere on the splitter.

but I'm not sure the problem is completely fixed.  I'm investigating.

I may need to fix the logic in OnClickThreadAndMessagePaneSpitter(),
now that I'm only calling that when the grippy is clicked.

I remember overhearing mscott and varada talking about this....
nevermind, I had it right.

the handled *only* gets called when you click on the grippy.
I was worried about clicking and dragging by the grippy, but that doesn't
fire the oncommand handler.

the logic is correct, the patch was correct.

fixed!  worth taking for 0.9, I'll drive that in tomorrow.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
I had the same bug 76566 which is marked as dup of this one and had the same fix
except that this wont work when you do the following. Collapse the splitter and
then pull it up instead of clicking it for opening it - the messages wont load
till you 'click and collapse' and 'click and open' it again. Also when you drag
the splitter to the bottom of the page to mimic a collapse the page still loads
because the logic is to handle only collapse. The way I tried was to get the
collapse attribute on the 'messagepanebox' and load the document only if it was
open - but the last scenario I described doesnt collapse. I have talked to
evaughan about this and have filed a bug on it.  Should I re-open this?
I didn't think about that case.

you're right, what we've got now is not complete.

moving the click handler to the grippy is part of the fix, but we'll need to add
a handler to the splitter to handle the case you mentioned.

reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Marking this bug as being blocked by 77172. 
When the state of the splitter changes from Dragging->Collapsed then the
messagepanebox (which is the bottom frame) has to have the collapsed attribute
set to true. This should be a mere extension of the existing functionality which
changes the "collapse" attribute of the grippy when the splitter reaches the
bottom. 
  This way we would be only dependant on the visibility of the messagepanebox
and not on the splitter, for the loading of messages.
Depends on: 77172
I've got a fix.

the "collapsed" attribute of the messagepanebox is correct, so we can rely on 
it.

so if the user clicks on the grippy, we call the grippy on click handler
and reverse the state for supression of msg display, since the state is
what is was when we clicked, not afterwards.

the splitter onclick handler doesn't get called when the user clicks and drags
the splitter open, but the onmouseup handler does get called.

so I added that, and set the state for supression to be the actual collapsed
state of the box, which is the state when we released the mouse.

this fixes the following cases that I could think of:

click on grippy to close:  suppress, since the box was not collapsed
click on grippy to open:   don't supress, since the box was collapsed
click and drag the splitter to open:  don't supress, since the box is not 
collapsed
click on a closed splitter and release:  supress, since the box is still 
collapsed

the only problem is having an onclick handler on the grippy and an onmouseup
for the splitter means if you click on the grippy, both get fired.
but since the order is onmouseup on the splitter and then onclick on the grippy,
everything works fine.

here comes the patch.
Status: REOPENED → ASSIGNED
varada, can you review and test before I go for a super review?
tested and reviewed the patch and it works on all boundary conditions. so until
QA or mscott comes up with a new splitter or grippy usage scenario :-) this
should more than suffice.
r=varada
Seth, you rock! I'd really like to see this land on the MOZILLA_0_9_BRANCH
Whiteboard: would like for 0.9
Whiteboard: would like for 0.9 → Have Fix - would like for 0.9
sr=bienvenu
a= asa@mozilla.org (on behalf of drivers) for checkin to the 0.9 branch.  Thanks
again Seth.
fixed (on the trunk).
Status: ASSIGNED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
*** Bug 78285 has been marked as a duplicate of this bug. ***
OK using may02 commercial trunk build: win98, mac OS 9.0, linux rh6.2
Status: RESOLVED → VERIFIED
*** Bug 78565 has been marked as a duplicate of this bug. ***
This didn't quite apply cleanly to the 0.9 branch but I think I've merged it
properly.  Seth, can you look at these and see if it looks OK to you?

Here's the first rejection:

[blizzard@decade content]$ cat mail3PaneWindowVertLayout.xul.rej 
***************
*** 155,162 ****
        </box>
        <!-- if you change this id, please change
GetThreadAndMessagePaneSplitter() and MsgToggleMessagePane() -->
        <splitter id="threadpane-splitter" collapse="after" persist="state"
           orient="vertical" autostretch="never">
-         <grippy onclick="OnClickThreadAndMessagePaneSplitter()" />
        </splitter>
        <!-- msg header view -->
        <box id="messagepanebox" orient="vertical" flex="4" persist="collapsed
height" class="window-focusborder" focusring="false">
--- 155,163 ----
        </box>
        <!-- if you change this id, please change
GetThreadAndMessagePaneSplitter() and MsgToggleMessagePane() -->
        <splitter id="threadpane-splitter" collapse="after" persist="state"
+        onmouseup="OnMouseUpThreadAndMessagePaneSplitter()"
           orient="vertical" autostretch="never">
+         <grippy onclick="OnClickThreadAndMessagePaneSplitterGrippy()"/>
        </splitter>
        <!-- msg header view -->
        <box id="messagepanebox" orient="vertical" flex="4" persist="collapsed
height" class="window-focusborder" focusring="false">

which ended up looking like this:

Index: mail3PaneWindowVertLayout.xul
===================================================================
RCS file:
/cvsroot/mozilla/mailnews/base/resources/content/mail3PaneWindowVertLayout.xul,v
retrieving revision 1.47
diff -u -r1.47 mail3PaneWindowVertLayout.xul
--- mail3PaneWindowVertLayout.xul       2001/04/22 16:40:52     1.47
+++ mail3PaneWindowVertLayout.xul       2001/05/03 00:19:50
@@ -154,8 +154,9 @@
       </box>
       <!-- if you change this id, please change
GetThreadAndMessagePaneSplitter() and MsgToggleMessagePane() -->
       <splitter id="threadpane-splitter" collapse="after" persist="state"
-         onclick="OnClickThreadAndMessagePaneSplitter()" orient="vertical"
autostretch="never">
-        <grippy/>
+         onmouseup="OnMouseUpThreadAndMessagePaneSplitter()"
+         orient="vertical" autostretch="never">
+        <grippy onclick="OnClickThreadAndMessagePaneSplitterGrippy()"/>
       </splitter>
       <!-- msg header view -->
       <box id="messagepanebox" orient="vertical" flex="4" persist="collapsed
height" class="window-focusborder" focusring="false">

and:

[blizzard@decade content]$ cat messenger.xul.rej 
***************
*** 156,163 ****
  
        <!-- if you change this id, please change
GetThreadAndMessagePaneSplitter() and MsgToggleMessagePane() -->
        <splitter collapse="after" persist="state" 
                  id="threadpane-splitter" orient="vertical" autostretch="never">
-         <grippy onclick="OnClickThreadAndMessagePaneSplitter()"/>
        </splitter>
        
        <box id="messagepanebox" align="vertical" flex="3" persist="collapsed
height" class="window-focusborder" focusring="false"
onclick="contentAreaClick(event);" ondraggesture="nsDragAndDrop.startDrag(event,
contentAreaDNDObserver);">
--- 156,164 ----
  
        <!-- if you change this id, please change
GetThreadAndMessagePaneSplitter() and MsgToggleMessagePane() -->
        <splitter collapse="after" persist="state" 
+               onmouseup="OnMouseUpThreadAndMessagePaneSplitter()"
                  id="threadpane-splitter" orient="vertical" autostretch="never">
+         <grippy onclick="OnClickThreadAndMessagePaneSplitterGrippy()"/>
        </splitter>
        
        <box id="messagepanebox" align="vertical" flex="3" persist="collapsed
height" class="window-focusborder" focusring="false"
onclick="contentAreaClick(event);" ondraggesture="nsDragAndDrop.startDrag(event,
contentAreaDNDObserver);">

which ended up looking like this:

Index: messenger.xul
===================================================================
RCS file: /cvsroot/mozilla/mailnews/base/resources/content/messenger.xul,v
retrieving revision 1.203
diff -u -r1.203 messenger.xul
--- messenger.xul       2001/04/22 16:40:53     1.203
+++ messenger.xul       2001/05/03 00:20:29
@@ -155,9 +155,10 @@
       <outliner id="threadOutliner" flex="2" persist="height"
style="height:0px" context="threadPaneContext"/>
 
       <!-- if you change this id, please change
GetThreadAndMessagePaneSplitter() and MsgToggleMessagePane() -->
-      <splitter collapse="after" persist="state"
onclick="OnClickThreadAndMessagePaneSplitter()"
+      <splitter collapse="after" persist="state"
+                onmouseup="OnMouseUpThreadAndMessagePaneSplitter()"
                 id="threadpane-splitter" orient="vertical" autostretch="never">
-        <grippy/>
+        <grippy onclick="OnClickThreadAndMessagePaneSplitterGrippy()"/>
       </splitter>
       
       <box id="messagepanebox" align="vertical" flex="3" persist="collapsed
height" class="window-focusborder" focusring="false"
onclick="contentAreaClick(event);" ondraggesture="nsDragAndDrop.startDrag(event,
contentAreaDNDObserver);">

ah, there was another fix that went into the trunk after we branched.

let me go find the bug with the patch. 

thanks for helping me out, blizzard.
duh, it's this bug, the first patch.  let me review blizzard complete fix.
that looks right, blizzard.

the splitter has the onmouseup hander and the grippy has the onclick handler.

the test is to click on the grippy (to collapse the message pane) and then 
click and drag the splitter open to reveal the message pane.

if you are able to click on messages in the thread pane and load them, you are 
golden.
Checked into 0.9.  Thanks!
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: