83495 - [PATCH] quirks.css rules applying to XUL

Status: VERIFIED FIXED

Description

[David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8 (if account gets disabled due to email bounces, ask a bugzilla admin to reenable it)]

The logs I attached to bug 83482 suggest that rules in quirks.css (particularly
the costly body > :first-node, td > :first-node, and td > :last-node rules) are
being used when loading XUL documents.  This is performance problem and I don't
think it should be happening.

Comment 1

[rbs]

dbaron, isn't this due to the namespace problem (bug 35847). It just seems weird 
(and wasteful) to me that rules scoped under a namespace are being considered 
everywhere.

Comment 2

[David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8 (if account gets disabled due to email bounces, ask a bugzilla admin to reenable it)]

Partly -- see also bug 83497.  But we shouldn't be loading quirks.css at all for
XUL -- we don't need it.  The code that disables it for standard mode should be
kicking in for XUL as well.

Comment 3

[David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8 (if account gets disabled due to email bounces, ask a bugzilla admin to reenable it)]

I think the problem here is that the defaults are a little confused, and there's
no explicit call to |SetCompatibilityMode| for XUL.  The default
mCompatibilityMode in the pres context is standard mode, but the default in the
style set is for the quirks stylesheet to be enabled (I think).  Whenever
there's an explicit |SetCompatibilityMode| call to the pres context, it
enables/disables the quirks stylesheet in the style set, but if there's no
explicit call the style set and the pres context are out of sync.

At least I think that's what's going on.  Marc - any suggestions on how to fix
this?

Comment 4

[Marc Attinasi]

I think that nsXULDocument::CreateShell could just call
aContext->SetCompatibilityMode like the nsHTMLDocument does. That would be
easiest but might add a little bit of extra processing time as each XUL doc
would have to make a call into the presShell - definitely worth trying as it
might just be noise. 

Another way to go would be to default quirks.css to disabled instead of enabled
and then enable it only when needed. The problem with that is the quirk
stylesheet is loaded via an @import from ua.css, and we would probably have to
change this to only load it when we are told to enable it, which might slow down
HTML docs...

I'd try the first approach and see if there is an adverse performance impact.


Index: nsXULDocument.cpp
===================================================================
RCS file: /cvsroot/mozilla/content/xul/document/src/nsXULDocument.cpp,v
retrieving revision 1.407
diff -u -r1.407 nsXULDocument.cpp
--- nsXULDocument.cpp	2001/05/19 11:26:45	1.407
+++ nsXULDocument.cpp	2001/06/01 17:12:23
@@ -988,6 +988,9 @@
     mPresShells.AppendElement(shell);
     *aInstancePtrResult = shell; // addref implicit in CreateInstance()
 
+    // tell the context the mode we want (always standard)
+    aContext->SetCompatibilityMode(eCompatibility_Standard);
+
     return NS_OK;
 }
 

Comment 5

[Marc Attinasi]

Created attachment 36849 [details] [diff] [review]
Patch to keep quirk rules out of XUL

Comment 6

[Marc Attinasi]

I attached a patch that I believe fixes this. It just makes a call to set the
compatibility mode on XUL documents. There is also a small fix to the method on
the styleset that enables/disables the quirk style sheet: it breaks out of the
loop after it finds the quirk stylesheet since there can only be one.

I traced through it and the quirk stylesheet is disabled for XUL documents now.

Taking this (hope you don't mind, David). Looking for comments, r, sr

Comment 7

[David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8 (if account gets disabled due to email bounces, ask a bugzilla admin to reenable it)]

r=dbaron

Comment 8

[Marc Attinasi]

CC'ing hyatt for his approval

Comment 9

[David Hyatt]

enthusiastic sr=hyatt!

Comment 10

[Christopher Blizzard (:blizzard)]

a=blizzard on behalf of drivers for the trunk

Comment 11

[Marc Attinasi]

Fixed.

Comment 12

[Hixie (not reading bugmail)]

Rubber stamp verification.