scope all theme css files to relevant xbl files

VERIFIED FIXED in mozilla0.9.1

Status

SeaMonkey
Themes
P1
normal
VERIFIED FIXED
17 years ago
10 years ago

People

(Reporter: Joe Hewitt (gone), Assigned: Joe Hewitt (gone))

Tracking

Trunk
mozilla0.9.1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

17 years ago
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.
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

17 years ago
Priority: -- → P4
(Assignee)

Comment 1

17 years ago
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
(Assignee)

Comment 2

17 years ago
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

17 years ago
Themes Triage Team nsbeta1+
Keywords: nsbeta1
(Assignee)

Comment 4

17 years ago
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.  
Keywords: nsbeta1
Priority: P1 → P3

Comment 5

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

Comment 6

17 years ago
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.  
Keywords: nsbeta1
Priority: P3 → P1
Target Milestone: --- → mozilla0.9

Comment 7

17 years ago
Themes Triage Team Marking nsbeta1+
Keywords: nsbeta1 → nsbeta1+
(Assignee)

Updated

17 years ago
Target Milestone: mozilla0.9 → mozilla0.9.1
(Assignee)

Comment 8

17 years ago
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...
(Assignee)

Comment 9

17 years ago
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

17 years ago
I'd prefer a patch.
(Assignee)

Comment 11

17 years ago
Created attachment 33423 [details] [diff] [review]
here's yer patch
(Assignee)

Comment 12

17 years ago
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. 

Updated

17 years ago
Blocks: 7251

Comment 13

17 years ago
sr=hyatt

Comment 14

17 years ago
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

17 years ago
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
(Assignee)

Comment 16

17 years ago
fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 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.
(Assignee)

Comment 19

17 years ago
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

17 years ago
Verified on commerical builds (2001-05-31-08-trunk).
Status: RESOLVED → VERIFIED

Updated

17 years ago
No longer blocks: 7251

Updated

17 years ago
Blocks: 7251
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.