I was reading nsRuleNode to try and understand how it works a little better, and I started tweaking nsRuleNode::WalkRuleTree. I changed a few little things that I think might make it a little faster -- I'll attach a patch. There are still two things I don't understand: 1) Why do we need to check that |detail == eRuleNone| before skipping rule nodes that have the inherit bit set for the struct? Doesn't having the inherit bit set for a struct mean that they have nothing to contribute? (I noticed removing this check messed up table borders.) This is noted with an XXXldb in the patch. 2) [Unrelated to this patch] All of the Compute*Data functions begin with a check that means parent* isn't set correctly when aRuleDetail == eRuleFullMixed. How does this manage to work correctly when some of the properties are inherited?

I can explain better in person (want to spare my hands).

Hyatt couldn't figure out the answer to #1, and we found bugs when looking into #2.

I'm wondering if #1 is related to the table element's attribute mapping function.

I still need to investigate issue #1, but I'd be interested in review on this patch.

sr=hyatt

I made one more comment tweak in the patch: @@ -1423,16 +1447,23 @@ } else if (!startStruct && ((!isReset && (detail == eRuleNone || detail == eRulePartialInherited)) || detail == eRuleFullInherited)) { - // We specified no non-inherited information and neither did any of our parent rules. We set a bit - // along the branch from the highest node down to our node indicating that no non-inherited data - // was specified. + // We specified no non-inherited information and neither did any of + // our parent rules. + + // We set a bit along the branch from the highest node (ruleNode) + // down to our node (this) indicating that no non-inherited data was + // specified. This bit is guaranteed to be set already on the path + // from the highest node to the root node. (We can only set this + // bit if detail == eRuleNone because an explicit inherit value + // could override a non-inherited value higher in the rule tree.) if (detail == eRuleNone) PropagateNoneBit(bit, ruleNode);

Comment on attachment 59476 [details] [diff] [review] patch fixing bugs above and adding more comments r=hewitt

Fix checked in 2001-12-01 16:44 PDT. Split off issue #1 into bug 114100.

Comment on attachment 60060 [details] [diff] [review] patch to fix leak (and other) regression from previous patch sr=waterson

For the record, the performance optimization of lazy calls to the parent context's GetStyleData was restored in bug 113098.

Also, for the record, someone actually discovered this bug without reading the code: http://groups.google.com/groups?selm=k84q8.369%24Np3.51315%40news2-win.server.ntlworld.com