63246 - scope all theme css files to relevant xbl files

Status: VERIFIED FIXED

(SeaMonkey :: Themes, defect, P1)

Description

[Joe Hewitt (gone)]

Hyatt has long ago checked in the capability to scope a css file to an XBL file.
 This could be a big memory saver, as it would allow un-used css files from
being loaded at startup.  Only as an XBL file is loaded would the corresponding
css be loaded.

This task is not small.  It involves breaking up several files, and moving
around stylesheet loading directives.  For starters, the files in xpfe/global
like xulBindings.xml should probably be split up into more granular per-widget
files.  Most of the css files loaded on startup would simply contain a
-moz-binding property and nothing more.

If we want to go even further, we can move more of the structure of communicator
apps into xbl to prevent loading of their css if that component isn't used.  For
instance, if the sidebar were completely in XBL, each app wouldn't have to load
it until it was first displayed.

Comment 1

[Joe Hewitt (gone)]

I really really really want to do this for the next releases of Mozilla and
Netscape, so I'm marking P1.  It will be a bit of a pain, but I'm looking
forward to it.

Comment 2

[Joe Hewitt (gone)]

I'd like to add that I would also like to extend this beyond xbl files and say
that all applications should only load relevant css files. For instance, in
mail/news there is too much sharing of messenger.css.  There should be a more
granular breakdown of styles.

Comment 3

[hangas]

Themes Triage Team nsbeta1+

Comment 4

[Joe Hewitt (gone)]

I ran some tests using <?xml-stylesheet?> PI's in XBL documents, and the results
were saddening.  The css files didn't seem to load correctly.  

Comment 5

[Blake Ross]

What was the problem (e.g., how did they load incorrectly?)

Comment 6

[Joe Hewitt (gone)]

Ok, this one is active again.  I've talked to Hyatt quite a bit about the issue
and he helped me out by fixing a bug or two which prevented this from happening.
 I've done most of the work to scope global, and I plan on landing that first
and then doing communicator and other apps after global lands.

Initial tests are inconclusive as to whether this saves memory or not.  In the
end, the best argument for doing it is that it makes XPToolkit more scalable
because we can add more and more widgets and variations but we won't load them
until we use them.  

Comment 7

[hangas]

Themes Triage Team Marking nsbeta1+

Comment 8

[Joe Hewitt (gone)]

Preliminary measurements of startup time and memory footprint aren't impressive.
 I can't perceive an improvement with scoping turned on.  QA is running some
tests of their own, so maybe they'll turn up different results.

In the mean time, I'd like to give a heads up on the way I've re-organized the
xbl files in global/content.  I've broken up xulBindings.xml and it's brethren
into a set of files which are contained under global/content/bindings.  Each
file is named something like "button.xml", "menulist.xml", "tabcontrol.xml". 
There will be a 1-to-1 correspondence between these xbl files and files under
global/skin.

For instance, chrome://global/content/bindings/menu.xml contains all the the
bindings for the xul tags "menu", "menuitem", "menuseparator" and various class
variations.  All of these bindings are scoped to use the css file
chrome://global/skin/menu.css.  The rules in menu.css will ONLY affect the
bindings in menu.xml.  If you replace one of these bindings in some skin XBL, be
you must extend a binding from menu.xml.

Even without scoping, the benefit of this reorganization is a clearer perception
of what bindings we have and how they related to the skin CSS.  

Patch forthcoming...

Comment 9

[Joe Hewitt (gone)]

I need to land this by Friday, so I need to get some reviewing action going
quick.  Problem is, this is a typically bitchy one to review.  I just did a diff
and it comes out to around 11,000 lines.  Not very useful.  

For those of you who should probably be reviewing this (blake, ben, hyatt, jag),
what flavor do you want? Should I give you a build and/or a zip of the changed
files and/or a monster patch?

Comment 10

[Blake Ross]

I'd prefer a patch.

Comment 11

[Joe Hewitt (gone)]

Attachment: here's yer patch

Comment 12

[Joe Hewitt (gone)]

I gave QA a test build on friday and today they got me back some results.  Look
like there is a slight speedup on startup time and new window time, somewhere in
the order of 0.5 - 1 second. 

Comment 13

[David Hyatt]

sr=hyatt

Comment 14

[Peter "jag" Annema]

This stuff looks pretty much okay. When you land this I will move my new
browserBindings.xml to this scheme and make the appropriate changes.

Some comments (all nit-picks):

- Some of your files don't seem to be ending in newlines. In xul that's not
really a problem, just an annoyance when checking in.

- You seem to have copied the html/xhtml namespace declaration to all files,
even where it's not needed.

- There is inconsistency in use of url(URL) and url("URL") in the CSS. I guess
you're just copying/moving those lines, so not an issue for this bug, but it's
something we should try to get in one style.

- You've created a few new files without a license embedded. As far as I know
all files should have a license in them.

It looks okay, nothing jumps out as "weird". Most of this should just be moving
stuff around. I see you've rearranged things here and there to group selectors
together when they have the same body, which all seems to be okay too. r=jag,
but I'd appreciate it if someone else can give his/her r= on this too.

Have you talked to leaf about making some test builds to be put up on
ftp.mozilla.org?

Comment 15

[Blake Ross]

Anyone know if we can remove moz-collapsed? It's only used in 
toolbarBindings.xml.

Also:

-/* this is unfortunate. should be display="none" */

Can you leave this comment (above the rule for hidden)?  Maybe it'll kick 
someone (like me) into changing this syntax one day.

r=blake otherwise

Comment 16

[Joe Hewitt (gone)]

fixed.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Looks like something in this checkin broke the form submission alerts.  If you
have mozilla set to display an alert on an insecure form submission, the text
next to the "never ask me again" checkbox is no longer visible after this
checkin.  Filed that as bug 80600

Comment 18

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

when you landed outliner.xml, did you use the lastest outlinerBindinds.xml?

something changes seem to be missing.

Comment 19

[Joe Hewitt (gone)]

I originally created outliner.xml in early April, and then used bonsai to track
changes to the file and inserted them myself.  I may have missed something

Comment 20

[Patty Mac]

Verified on commerical builds (2001-05-31-08-trunk).