Closed Bug 391300 Opened 17 years ago Closed 17 years ago

Switching to mail mode does not restore collapsed elements properly

Categories

(Calendar :: Lightning Only, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: alta88, Assigned: michael.buettner)

Details

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6 XPCOMViewer/0.9.5
Build Identifier: 


Calendar mode button calls function showCalendarView(), here:
http://lxr.mozilla.org/mozilla1.8/source/calendar/lightning/content/messenger-overlay-sidebar.js#495
which collapses message pane, threadpane splitter, searchbox.

Mail mode button function ltnSwitch2Mail() here:
http://lxr.mozilla.org/mozilla1.8/source/calendar/lightning/content/messenger-overlay-toolbar.js#100
doesn't restore those items, so i couldn't view a selected message and the searchbox in the mail toolbar was gone.

just need to insert this code in ltnSwitch2Mail() to restore properly:

uncollapseElement(GetMessagePane());
uncollapseElement(document.getElementById("threadpane-splitter"));
var searchBox = findMailSearchBox();
if (searchBox) {
uncollapseElement(searchBox);
}


Reproducible: Always
Attachment #278315 - Flags: review+
Comment on attachment 278315 [details] [diff] [review]
Restore collapsed elements when switching back to mail mode

alta88, you probably wanted to ask for review? Clearing review-granted flag.
Attachment #278315 - Flags: review+
(In reply to comment #0)

Please provide detailed steps to reproduce. Using Thunderbird 2.0.0.7pre (20070823) and Lightning 0.7pre (2007082604) I don't see any elements that disappear after switching to calendar mode and back to mail mode.
In addition: What Thunderbird and Lightning version do you use?
Thunderbird/2.0.0.7pre ID:2007082304
Lightning 0.7pre (2007080305)

if you look in the code in comment #1, you will see where these elements are being set to collapsed=true and not set back.

again this is with the wide thread view and Stacked View extension.

i realize these are not official, but was thinking that in this case collapsing elements and counting on Tb to restore them might be not as good as explicitly setting them back to where they were found, esp since it's an easy fix.  i'll try to find another way if it's felt it isn't appropriate to fix this in lightning.

btw, i'd like to see these 2 views officially in Tb, and have put together some much better working code for them.  i'll see about making it work with lighting as well.

http://forums.mozillazine.org/viewtopic.php?t=578861
Comment on attachment 278315 [details] [diff] [review]
Restore collapsed elements when switching back to mail mode

Mickey should probably review this patch. But as he is busy right now, do not expect this review before 0.7
Attachment #278315 - Flags: review?(michael.buettner)
Assignee: nobody → alta88
MoreLayoutsforThunderbird has been released as an extension:
http://morelayoutsforthunderbird.mozdev.org/

i realize we're in rc1 but since this patch is truly as low risk as they come, i'd like to request if it could be included in 0.7.
(In reply to comment #7)
> i realize we're in rc1 but since this patch is truly as low risk as they come,
> i'd like to request if it could be included in 0.7.
Indeed this is a low-risk patch, but the fix is not the correct way to address the initial problem. Since we now have the dedicated calendar mode collapsing these elements is superfluous. In any case, this is too late in the game, it won't take the 0.7 train.
alta88, wouldn't it make sense to just remove the following lines from messenger-overlay-sidebar.js?

> collapseElement(GetMessagePane());
> collapseElement(document.getElementById("threadpane-splitter"));
> var searchBox = findMailSearchBox();
> if (searchBox) {
>   collapseElement(searchBox);
> }
Comment on attachment 278315 [details] [diff] [review]
Restore collapsed elements when switching back to mail mode

r- based on my previous comments.
Attachment #278315 - Flags: review?(michael.buettner) → review-
(In reply to comment #9)
> alta88, wouldn't it make sense to just remove the following lines from
> messenger-overlay-sidebar.js?
> 
> > collapseElement(GetMessagePane());
> > collapseElement(document.getElementById("threadpane-splitter"));
> > var searchBox = findMailSearchBox();
> > if (searchBox) {
> >   collapseElement(searchBox);
> > }
> 

indeed it would.  if you are confirming that Lightning no longer needs to do the collapse, well then the uncollapse is also unnecessary..

do you prefer i submit a patch for the removal, or can you take care of it?

thanks.
Attached patch patch v1Splinter Review
This patch takes care of the required modification as suggested in my previous comment.
Assignee: alta88 → michael.buettner
Attachment #278315 - Attachment is obsolete: true
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #285206 - Flags: review?(philipp)
Comment on attachment 285206 [details] [diff] [review]
patch v1

Looks good, r=philipp
Attachment #285206 - Flags: review?(philipp) → review+
Whiteboard: [checkin-needed after 0.7]
patch checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed after 0.7]
Target Milestone: --- → 0.8
Michael, is it a correct behaviour (with Lightning 2007102923)?:
1)Create three draft e-mails and save them
2)Add labels to them (i.e. two of them "to do", and the last one "personal")
3)Group them by label

With Lighting 0.7 when you expand the view clicking '+' but you don't select an e-mail and then switch to calendar mode and back to mail mode you can see everything untouched (All e-mails are visible). When expand the view clicking '+' and you select an e-mail and switch to calendar mode and back to mail mode you can't see any e-mails.

And now with Lightning 2007102923:
1)When you expand the view clicking '+' but you don't select an e-mail and then switch to calendar mode and back to mail mode you can't see any e-mails
2)When you expand the view clicking '+' and select an e-mail and switch to calendar mode and back to mail mode you can see the selected e-mail and the expanded group from where you selected the e-mail. The other group is not expanded (but it was expanded before switching modes).
I am verifying this fix against lightning 2007121104 build it works for the mail folders but does not work for newsgroups. The folders are expanded but the message that was selected before going to calendar view is unselected after coming back. Otherwise everything else is OK. Thanks.
> [...] the message that was selected before going to calendar view
> is unselected after coming back.

parvata, perhaps your and Omar's concerns will be addressed by bug 398900.
You need to log in before you can comment on or make changes to this bug.