66475 - Clean up mailnews support for alt layout

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[timeless]

Tracker for another pie in a different sky.

Comment 1

[timeless]

erm, it was supposed to be assigned to me.

Comment 2

[timeless]

Attachment: zip of new files and diffs that i worked from

Comment 3

[timeless]

Attachment: DumpDOM and Dump Scripts bound to objects

Comment 4

[[e-mail]]

Will this include folder/thread pane dependencies? I tried to delete the thread
pane from a copy of mail3PaneWindowVertLayout.xul and found that the status bar
update depends on it :-(

Comment 5

[timeless]

yeah i'm going to try and make everything work cleanly and correctly.

we can even do message center.

Comment 6

[Matthew T (active 1999-2002)]

Cool. I was running around today trying to find someone to do this, not realizing 
that it was already being done. :-) The only difference between a three-pane 
window, a standalone message window, and the Message Center, should be which 
panes are turned on in the `View' > `Show' submenu.

See also <http://deja.com/=dnc/getdoc.xp?AN=603946168>.

Comment 7

[scottputterman]

Seriously, for the same reason as I said in a different bug, one reason to not
get rid of the separate standalone window is due to time to load. The standalone
is pretty small right now and will probably load faster than a window with all
of the chrome hidden. I'm not against it if you find a good way to bring these
windows together (especially the alt 3pane and regular 3 pane) because it will
make everything easier to maintain, but please don't try to force things
together just because there's not much difference in the UI.

Comment 8

[timeless]

hrm, stephend: could you run performance tests on these files? i know i need to 
upload newer versions, i'll do that after class.

Comment 9

[Matthew T (active 1999-2002)]

If there are noticable performance effects, all that means is that turning a 
standalone window into a three-pane window should be done in a different way by 
the back end -- instead of just toggling the display property, you would have 
to tear down the standalone content and create the three-pane content in the 
same window. The user interface would be the same.

Comment 10

[timeless]

putterman/mpt please spam a newsgroup, I'm not setting display:none, i'm just 
not _using_ things that i don't need. and I can easily take all of the elements 
used by single pane and have them included by it and the general 3pane 
template, it still saves cost.

Comment 11

[timeless]

Attachment: current work in progress

Comment 12

[timeless]

ok this is certainly not complete, but you get a few bonuses from the rewrite:
All toolbars in [standalone]messageWindow are in one toolbox, so they collapse 
to one row :-)

Most functionality works, i need to move some scripts out of messageWindow into 
messageWindowOverlay due to initialization features:
file1:
<script src="foo"/>
file2.xul:
<?xul-overlay file1?>
<window><script src="bar"/></window>

foo executes after bar which is probably why navigatorOverlay was created [to 
force scripts to init later].

i still need to rearrange some things but i'm not sure how much.  I'd 
appreciate it if people would test this version.

you can either place 
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=23530 as 
mozilla/res/samples/DumpDOM.js or remove the reference to it [it's currently 
unused]

messageWindow no longer pulls in things that are certainly only for threepane.

Comment 13

[scottputterman]

ok, so you aren't combining the windows. There's still a separate standalone
window with a js file and a separate 3 pane with a js file.  Then everything
I've said about performance won't be affected.  However, are you putting a
sidebar in the standalone?  Wouldn't many of the same complaints that prevented
it from going into the mail compose window be the same for this?  My opinion is
that we shouldn't put the sidebar in this window for performance reasons.  If
I'm reading the xul wrong then ignore me.

Comment 14

[timeless]

it's in there now, if people don't want it, i can easily remove it.  feel free 
to get a response from the newsgroups.

Comment 15

[scottputterman]

I don't think the sidebar should be in it.  Until we get message navigation in
this is still a window that won't remain open very long.  If for some reason the
sidebar stays in, I'd be interested in seeing a screen shot. Without having seen
it, I think it might look strange with the message's headers stretched out over
the sidebar and the message.

Comment 16

[[e-mail]]

Personally I see nothing wrong with including the sidebar if it defaults to
being collapsed, as with Editor.

Comment 17

[scottputterman]

:-) I'll start a news group posting on this. Where were all you guys when we
wanted to put a sidebar in the message compose window defaulted to closed?

Comment 18

[(not reading, please use [e-mail] instead)]

before we add a sidebar, I'd need to be convinced (with performance numbers)
that it doesn't slow us down.

Comment 19

[jglick]

I would not want to see a sidebar in Mail Standalone, unless it was Hidden (not 
just collapsed) by default and didn't effect performance.

Comment 20

[timeless]

i won't check it in if it 'affects' performance. Defaulting to not visible 
[instead of just collapsed] is fine with me.  filed Bug 67609 to siphon off 
discussion.

Comment 21

[[e-mail]]

Attachment: Message Center for Netscape 6 (no localization)

Comment 22

[timeless]

Attachment: preliminary sync

Comment 23

[Mark Anderson]

Ok.  Attaching a new patch that's cleaned up for the <box> conversion.  It's not 
tested; it's just converted.  I checked all the pertinent overlays, and the 
layout I have in this file matches up with that.  However, there are some 
interesting anomalies wherein the orientation of certain boxes (for instance, 
id="messagepanebox") are different than they are in the trunk mailnews stuff.  
Dunno if this is intentional.  As I said, I just did a straight conversion.

Comment 24

[Mark Anderson]

Attachment: New patch, with changes for 73671 taken into account

Comment 25

[Håkan Waara]

Why are the files named timeless[1|2|3|4].xul?

Comment 26

[[e-mail]]

The patch would have to be rewritten anyway if View/Folders (bug 19147) lands.

Comment 27

[Håkan Waara]

I don't understand this bug. What is it about? What does it fix?

Is it just code reordering or does it actually *fix* any bugs?

Would be good with some clarification.   Thanks.

Comment 28

[Chase Tingley]

linux 'patch' freaks out on the latest version: 

patch: **** malformed patch at line 268: Index: base/resources/content/timeless1.xul

has anyone else gotten it to apply?

Comment 29

[timeless]

there have been many bugs about fixing the alt layout because most people 
forget. this meakes it so you wouldn't have to fix the alt layout since they'd 
share code.  (all views would share nearly all code.)

given mozilla's structure this is the thing to do (asside from building xbl 
widgets).

as for bitrotting, that's what this patch does best. If you guys have a 
suggested time for me to reincarnate it, i'll wait, otherwise i'm probably 
better off getting it rehabbed and landed so you can work based on it.

filenames are of course not permanent.

that's a really whacky place for patch to fail. If you have a program that can 
s/^+// you might do that instead. They're all knew files...  I'll probably make 
a diff against my old patch at some point, but right now bugzilla isn't 
accepting the patch i have for sspitzer afterthefact.

Comment 30

[Håkan Waara]

I noticed that this bug is targeted at 0.9.3

This is a big architecture change, and I wouldn't expect it to go in without
thorough review and after folder_outliner landing (which is also a big arch.
change).

Furthermore, this would need lots of testing since it's probably not exempt from
regressions. ;)

Looks like good work, but I suggest you push it beyond 0.9.3

Comment 31

[[e-mail]]

I think this looks as if it might dump too much into messageWindow.xul :-(
While I agree that most of the portions common to the messenger windows (e.g.
accountCentralBox, threadPaneSplitter) should go into a new overlay which I
think should be called messengerOverlay.xul, I think that the portions which are
also common to messageWindow.xul, e.g. emailAddressPopup, allHeadersPopup should
go into mailWindowOverlay.xul. A box which I think should be called
folderpanebox that contains the sidebar header (? - 4.x used a tree header) and
the folder outliner/tree should go in folderPane.xul, the threadpanebox should
go in threadPane.xul, the messagepanebox and messagepane should go in
msgHdrViewOverlay.xul. Also I think messageWindow could have its own status bar
instead of trying to share it with the messenger windows.
These comparatively minor adjustments should be easy to arrange and review.

Comment 32

[[e-mail]]

Attachment: What I ended up with.

Comment 33

[[e-mail]]

What I did:

Created messengerOverlay.xul for:
overlays
scripts
commandset mailCommands
keyset mailKeys
popupset mailPopups (was aTooltipSet)
toolbox mailToolbarToolbox
splitter threadpane-splitter

Moved to folderPane.xul (not loaded by search dialog):
popup folderPaneContext
vbox folderpanebox

Moved to threadPane.xul (not loaded by message window):
popup threadPaneContext

Moved to msgHdrViewOverlay.xul:
popup messagePaneContext
popup emailAddressPopup (currently not in an overlay)
popup allHeadersPopup
popup attachmentTreeTooltip
vbox messagepanebox

Found 3 bugs while doing this:
1. mail3PaneWindow.js depends on ContentAreaClick.js
2. messageWindow.js strongly depends on mailWindow.js
3. Account Central doesn't work with alternate message pane collapsed.

Comment 34

[chris hofmann]

time for 0.9.4 has run out.  try for 0.9.5. thanks -chofmann

Comment 35

[timeless]

neil has been working on it

Comment 36

[(not reading, please use [e-mail] instead)]

neil, see bug #105542 for alternate approach.

I'm hoping to get rid of mail3PaneWindowVertLayout.xul, and do the pane 
configuration dynamically.

Comment 37

[[e-mail]]

Seth, I've got a patch that just brings messenger.xul and
mail3PaneVertLayout.xul into sync so that all the ids etc match. There are also
some changes to Show/Hide Account Central and Switch Pane Focus that are
affected by this. For this patch I have not attempted to make allowances for
messengerWindow.xul duplication.

Comment 38

[[e-mail]]

Attachment: Patch to sync

Comment 39

[Jacek Piskozub]

Is this bug still meaningful after bug 105542 ["dynamically generate 3 pane
layout, so we can get rid of the alt3-pane xul"] was fixed?

Comment 40

[[e-mail]]

Marking as effectively fixed via bug 105542.