Closed Bug 63246 Opened 19 years ago Closed 19 years ago
scope all theme css files to relevant xbl files
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.
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.
Priority: P4 → P1
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.
Themes Triage Team nsbeta1+
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.
Priority: P1 → P3
What was the problem (e.g., how did they load incorrectly?)
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.
Priority: P3 → P1
Target Milestone: --- → mozilla0.9
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...
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?
I'd prefer a patch.
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.
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?
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
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
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
when you landed outliner.xml, did you use the lastest outlinerBindinds.xml? something changes seem to be missing.
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
Verified on commerical builds (2001-05-31-08-trunk).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.