114100 - investigate (detail==eRuleNone) check in nsRuleNode::WalkRuleTree

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P4)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This bug is split off from bug 111815.

In nsRuleNode::WalkRuleTree, I'm not usre why 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 function from the patch for bug 111815.

Removing this check (which would slightly improve performance) messes up table
borders.  I'm wondering if this issue is due to a bug in the attribute mapping
function for the table element.

Comment 1

[Madhur Bhatia]

cc'ing myself

Comment 2

[Aaron]

I was the one who submitted this, but I cannot use firefox to access the site, because it will crash one out of every two times I go to it on average.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #0)
> Removing this check (which would slightly improve performance) messes up table
> borders.  I'm wondering if this issue is due to a bug in the attribute mapping
> function for the table element.

In other words, fixing this may depend on bug 211636.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

This passes reftests, but I'd still like to think about it a little more.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Did you figure out what the table borders issue was?

Comment 6

[Boris Zbarsky [:bzbarsky]]

To be clear, the review is waiting on an answer to comment 5.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I don't know, but there have been substantial improvements to the table attribute mapping functions in the past seven and a half years.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 371559 [details] [diff] [review]
patch

Fair enough.  This looks fine to me.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://hg.mozilla.org/mozilla-central/rev/60b208d3f9b8