72208 - Starting app with sidebar closed creates panels, should wait until first open

Status: RESOLVED FIXED

(SeaMonkey :: Sidebar, defect)

Description

[Mike Pinkerton (not reading bugmail)]

I've noticed (because of an assert in a treewidget frame) that if you have the 
sidebar enabled but collapsed at startup, it goes ahead and creates the panel 
anyway even though you can't see it. This has an impact on both startup and new 
window performance.

What we should do is have it not load the panel until you expand the sidebar for 
the first time, and then it can keep it alive from that point onward to retain 
state, etc when collapsed.

Comment 1

[Paul Chen]

nav triage team:

not going to happen for beta, marking nsbeta1-

Comment 2

[John Morrison]

Probably not enough to change anyone's mind, but I went and measured this on 
the test machine for win98 - PII 266 / 128MB, and here's what I have:

Average time for 10 consecutive starts (very little scatter), 
with sidebar ...
  Collapsed       Open (to 'News')         Hidden
    7.74	       7.71	            7.50

So, that's about 2/10ths of a second, but only about 3% overall.

Comment 3

[Mike Pinkerton (not reading bugmail)]

jrgm, what if that sidebar panel we're creating is AIM? that would unnecessarily 
drag in all the aim libraries for no good reason.

which panel did you use to do your tests?

Comment 4

[John Morrison]

Thanks for reminding me: I was going to come back to this again to mention that 
perhaps 'on startup' was the wrong way to think about this 0.2 second cost. It 
may be the case that we pay this cost on every new window, which would be much 
more significant in proportion.

But to answer your questions: I was using the bookmarks panel for my test, 
with the default bookmark list that ships with Netscape. 

As for AIM, I believe it is the case that we pull in the AIM dll's on startup
no matter what (although maybe it is the case that it is the presence of the 
sidebar panel that triggers this loading). 

(There's another problem with AIM for which I will file a bug in bugscape, but 
I'll mention it here: if the AIM sidebar panel is the selected panel, but the 
sidebar is collapsed and I have not done the initial signup dance, then every
time I create a new navigator window, an AIM dialog window appears out of 
nowhere, asking me to signin, etc.)

Comment 5

[Blake Ross]

--> me

Comment 6

[Peter Trudelle]

Mass moving lower-priority bugs to 0.9.6 (with Blake's pre-consent) to make room
for remaining 0.9.4/eMojo bugs and MachV planning, performance and feature work.
 If anyone disagrees with the new target, please let me know.

Comment 7

[Blake Ross]

Attachment: patch

Comment 8

[Blake Ross]

Samir, review?

Comment 9

[Samir Gehani]

Attachment: Handle the case when the sidebar is hidden as well. r=sgehani for this modified patch.

Comment 10

[Blake Ross]

Samir: when the sidebar is totally hidden (i.e. closed), we don't even create
the frames for it or anything, so as far as I know we already don't load the
sidebar if it's closed.  An alert in search-panel.xul's onload (which I was
using to test) seems to confirm that.

Comment 11

[Samir Gehani]

Good point.  Although we don't need to do the RDF initialization stuff if the
sidebar is hidden so the modified patch doesn't hurt.  (sidebar_overlay_init()
itself gets called even when hidden with your patch, not when collapsed though,
but arguably does little.)  This is a nit I don't fel strongly about so
r=sgehani on either patch.

Comment 12

[Blake Ross]

I didn't see sidebar_overlay_init() getting called when the sidebar was
completely hidden so I checked in the patch as-is.