Closed
Bug 6625
Opened 26 years ago
Closed 24 years ago
Redundant rules in ua.css
Categories
(Core :: CSS Parsing and Computation, defect, P2)
Core
CSS Parsing and Computation
Tracking
()
VERIFIED
FIXED
M18
People
(Reporter: ian, Assigned: ian)
References
()
Details
(Keywords: perf)
Attachments
(1 file)
(deleted),
text/plain
|
Details |
The following rules are redundant in ua.css:
Lines 192, 198, 216, 221: The "a" in the selector is redundant and should be
removed. This is especially important for future proofing (eventually,
elements other than "a" will be able to be links).
Lines 204 and 226: The "a" in the selector should be changed for ":link" and
":visited", so that the selectors read
:link:active, :visited:active
...instead of
a:active
This is also for future proofing (see first comment).
Lines 192-203: the "display", "text-decoration" and "cursor" declarations of
the :link and :visited rules could be merged, like this:
:link, :visited {
display: inline;
text-decoration: underline;
cursor: pointer;
}
Lines 204-209: the "display", "text-decoration" and "cursor" declarations
can be completely removed as they are specified by the :link and :visited
rules, because the :active pseudo-class is concurrent with the :link and
:visited pseudo-classes.
Lines 219, 224, 229, 234: the "text-decoration:none" declarations will have
no effect as the underlining from the surrounding links would "override"
it anyway.
Lines 216-235: All these rules can actually be collapsed into a single rule:
:link img, :visited img {
border: 2px solid;
}
This is because the :active pseudo-class is concurrent with the :link and
:visited pseudo-classes, and the other declarations can be removed as
described in the comments above.
Many lines: The "display:inline" declarations on many of the lines are
completely superfluous as that is the default value of the display property.
Some other changes that could be made to increase the efficiency and decrease
the size of the ua.css:
1. line-height:normal does not need to be defined in the body; it is the default
2. Instead of defining every hx element separately, like
h1 {
display: block;
font-size: xx-large;
font-weight: bold;
margin: 1em 0;
}
h2 {
display: block;
font-size: x-large;
font-weight: bold;
margin: 1em 0;
}
it would be better to use
h1, h2, h3, h4, h5, h6 {
display: block;
margin: 1em 0;
font-weight: bold;
}
and then define the font size for each header separately.
3. Elements without any styles applied to them don't need to be included at
all. This would include abbr, acronym, del, dfn, ins, q, span; spacer, wbr, :-
moz-text, img; :-moz-line-frame, and :-moz-letter-frame.
4. Instead of defining each to be displayed as a block-level element
seperately, group :cell-content, :fieldset-content, :button-content, :label-
content together with {display: block}. Do the same with {display: none}
elements.
5. Use shorthand. Instead of
{padding-left : 2px;
padding-right : 2px;
padding-top : 1px;
padding-bottom: 1px;}
use
{padding: 1px 2px;}
6. For input[type=hidden], dont use {border-none}. The {visibility: none} takes
care of the border.
(Another thing: a 1px border on iframe looks better than a 2px border. The
entire interface of apprunner looks much cleaner.)
Another thing:
There are two iframe rules. They should be combined to read
iframe {
background-color: white;
border: 2px solid black;
}
or even better, border: 1px solid black.
Updated•25 years ago
|
Target Milestone: M10 → M11
Updated•25 years ago
|
Target Milestone: M11 → M20
Comment 4•25 years ago
|
||
Deferring this. These are all good suggestions, but as time goes by the file
will get messy again. I'll make a cleanup pass shortly before ship.
Comment 5•25 years ago
|
||
Regarding comment no 2 from Additional Comments From bloviate@yahoo.com
05/29/99 19:13:
In CSS1 specification it is noted that the em is a relative unit and will be
inherited as the computed value therefor also the margin setting should be in
the seperate H1, H2 etc. rules
Comment 6•25 years ago
|
||
Reassigning peterl's bugs to myself.
Comment 7•25 years ago
|
||
Accepting peterl's bugs that have a Target Milestone
Comment 8•25 years ago
|
||
Let's consider this a Time&Space problem... Reassigned to attinasi.
FYI, we have 270 rules in html.css and 78 in xul.css.
Assignee: pierre → attinasi
Status: ASSIGNED → NEW
Comment 9•25 years ago
|
||
I believe that Peter's comment that this need to be cleaned-up before shipping
is valid, however I also think we should make any valid changes incrementally,
as we come across them, so we are getting the most possible testing time with
the most relevent rules in place (reduced risk). Thus, I'll start applying these
changes soon after beta.
NOTE: reducing the number and size of rules helps lower the memory impact of the
StyleSytem and can thus be considered an optimization, which is a benefit in
addition to that of increasing the maintainability of the document by reducing
redundancy.
Status: NEW → ASSIGNED
Target Milestone: M20 → M16
Updated•25 years ago
|
Target Milestone: M16 → M20
Assignee | ||
Comment 10•25 years ago
|
||
Marc: Would you prefer if I went through the html.css file and filed bugs with
diffs one change at a time, or would you rather I went through html.css and
quirks.css and neatened them both up in one go, and then attached the new files
to this bug (or the quirks.css bug) directly?
Comment 11•25 years ago
|
||
If you find quirk-rules that should be fixed for PR2 then I suggest you pull
them out into separate (new) bugs. General cleanup can go into one big change
for this bug. My reasoning is that the PDT team is more likely to approve
changes that are small and discrete, and some of the changes are not related to
quirk vs. standard mode rule migrations (i.e. they are general cleanup). Sound
OK Ian? Thanks!
Comment 12•24 years ago
|
||
OK - I guess we shouldn't put this off any longer, it is time to clean the mess
up and watch the changes that get checked in from now through RTM
Keywords: nsbeta3
Assignee | ||
Comment 13•24 years ago
|
||
The easiest way to do this would be to lock that file, then I could spend a day
cleaning it up and browsing to make sure nothing broke, and then we could get
the file checked in. But this requires the file to have absolutely no changes
made to it, otherwise they will be lost.
Alternatively, we can do this one patch at a time, but that would take ages...
Comment 14•24 years ago
|
||
For performance, the key is to reduce the number of selectors.
Comment 15•24 years ago
|
||
Do we want to optimize the rules (selectors) as well as remove the redundant
ones? That seems like a higher risk proposal, and maybe that should be a
seperate task done after the redundancy is eliminated.
Ian, I'll look into locking a file: presumable html.css and quirk.css, or do
you want to attack xu.css too? Even if we cannot lock them, I think the rate of
change is very low now (last change to html.css was mid-June) so we should be
able to manage the update with little more than a heads-up email and posting to
the community.
Assignee | ||
Comment 16•24 years ago
|
||
I don't know enough about the magic behind the xul selectors to edit xul.css
as well, so best leave that one alone.
Similarly for parts of html.css like this:
:-moz-line-frame {
}
:-moz-letter-frame {
}
I assume I either just leave those alone or put them in a comment block at
the end of the file?
Comment 17•24 years ago
|
||
Hmmm. What good is an empty rule? Maybe it is a reminder? In that case a
comment-block is probably best.
Also, since Hyatt has considerable expertiese, and since he is best equipped to
handle the xul.css file, I'm CC'ing him on this (I believe he has already tuned
xul.css anyway).
Comment 18•24 years ago
|
||
Approving for nsbeta3: this needs to be doen prior to beta3 completion and then
locked down for FCS.
Whiteboard: [nsbeta3+]
Assignee | ||
Comment 19•24 years ago
|
||
Marc - I suggest we postpone this until just after nsbeta2 ships. Until then I
will be very busy blessing builds and so on.
Once nsbeta2 ships, we'll need to announce the ua.css|html.css|quirks.css
lockdown and once that is done I can start going through the stylesheets.
Also, when you have a moment could you list the best ways of optimising the
stylesheets? David mentioned using grouping as much as possible, for example.
Comment 20•24 years ago
|
||
Ian, I agree that this should wait a while - in fact, we could wait until the
very end of the beta3 cycle (we should leave a week for testing though).
I really don't think we should be optimizing too much - the focus should be on
removing redundancy and eliminating rules that have no affect. That will be a
performance gain too. The more we restructure rules the more risk we take on, so
we should minimize that activity and focus on size reduction by elimination of
cruft and duplication.
Assignee | ||
Comment 21•24 years ago
|
||
Even though the risk level is low, it is wide ranging since this affects _all_
HTML content. So I'd rather we did it nearer the start of the beta3 cycle.
Also, if we leave it to the end then I'll be doing beta3 blessing so we'll have
run out of time again. ;-)
Question: Is it a bad thing to have more stylesheets linked in from ua.css? I
was considering moving the viewsource styles out into another file (they are a
bit out of place in html.css). Would that be a bad thing?
Comment 22•24 years ago
|
||
Each imported stylesheet has to be file-opened, but other than that I don't
think that there is a problem with importing viewsource.css into us.css (for
example).
As for the timing comments - makes perfect sense :)
Updated•24 years ago
|
Priority: P3 → P2
Assignee | ||
Comment 23•24 years ago
|
||
I'm on it.
Assignee: attinasi → py8ieh=bugzilla
Status: ASSIGNED → NEW
Target Milestone: M20 → M18
Assignee | ||
Comment 24•24 years ago
|
||
I'm also trying to move lots of the quirky rules in html.css to quirk.css. We'll
see how that goes.
Status: NEW → ASSIGNED
Assignee | ||
Updated•24 years ago
|
Priority: P2 → P1
Comment 25•24 years ago
|
||
PDT would like to know: is this going to make a real impact on performance or
footprint? Or are we just beautifying? Cleanup can wait until after seamonkey.
Assignee | ||
Comment 26•24 years ago
|
||
It will have some impact; how much I do not know, it would need measuring.
The work is almost all done anyway, I just need to check it through the top100
and the various CSS test suites before passing it back to Marc for checkin.
Comment 27•24 years ago
|
||
The potential for speed and size improvements are good. Rules take up a lot of
memory, and processing rules takes a lot of time in page layout and event
handling, so by lowering the number of rules we should be able to get both a
speed-up and a memory savings, proportional to the percentage decrease in the
number of rules - roughly speaking.
Comment 28•24 years ago
|
||
There are some very inefficient rules in html.css that would have an impact on
style resolution performance of form elements... e.g., stuff like
td input {
}
slows down all input controls. This is bad.
Assignee | ||
Comment 29•24 years ago
|
||
Part of my fix has been to move these rules into quirk.css. Thus only pages
using compat mode will be affected. I may remove _some_ of those rules
altogether, depending on whether I can find why they were put in in the first
place.
Assignee | ||
Comment 30•24 years ago
|
||
Ok, I'm about to mail out the proposed new files to everyone who has claimed
they want to review the changes before this is checked in. ;-)
Comment 31•24 years ago
|
||
Ian, ua.css is at risk of not making the commercial product. We really need the
changes to be stable and landed very, very soon. Once we start stabilizing for
RTM, only super serious P1's will make the cut.
Priority: P1 → P2
Whiteboard: [nsbeta3+] [FIX IN HAND] → [nsbeta3+] [FIX IN HAND][PDTP2]
Comment 32•24 years ago
|
||
Ian, wasn't this checked in? Please reopen if I'm mistaken, thanks.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 33•24 years ago
|
||
blake: Yes, it was checked in. However, I had to check that what was checked in
was what I had written before marking it fixed. Since you did not know what my
final files looked like, you could not check that, so should not have marked
it fixed.
Having said that: FIXED by halving the size of html.css. Bloat is down a not
inconsiderable amount according to Marc's figures.
ChrisD: To verify, examine the ua.css, quirk.css and html.css files that are
present in the distribution and check that they are now considerably smaller
than in previous days.
Comment 34•24 years ago
|
||
Per Ian's suggestion, verified that html.css is approximately cut in half.
Marking bug verified
Status: RESOLVED → VERIFIED
Assignee | ||
Updated•24 years ago
|
Whiteboard: [nsbeta3+] [FIX IN HAND][PDTP2]
You need to log in
before you can comment on or make changes to this bug.
Description
•