147871 - .foo:before matches all mathml elements

Status: VERIFIED FIXED

(Core :: MathML, defect)

Description

[David A. Madore]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.0+) Gecko/20020523
BuildID:    20020523

See attached URL: the CSS stylesheet defines the rule .bug:before { content:
"BUG" } meaning that every element with class "bug" should prepend the inline
content "BUG".  In HTML this works correctly, but in MathML the content "BUG" is
prepended before _every_ element.

Reproducible: Always
Steps to Reproduce:
View http://www.eleves.ens.fr:8080/home/madore/.test/mathmlbug.xml



Actual Results:  (described in the page)

Expected Results:  (described in the page)

Comment 1

[Hixie (not reading bugmail)]

That's very... weird. It's like we're not correctly determining that mathml
elements don't match the selector or something.

Comment 2

[rbs]

Likely a |selectorMatches| problem indeed. The testcase is suggesting that all
MathML elements seem to be erroneously treated as if they have class="bug".

Comment 3

[rbs]

Seems to be a bug in the handling of :before/:after pseudos. With something else,
e.g., .bug { color: red }, the rule doesn't match.

Comment 4

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

I see exactly where the problem comes from :

Excerpt from |SelectorMatches()| :

  nsAtomList* classList = aSelector->mClassList;
  while (nsnull != classList) {
    if (localTrue ==
          (NS_COMFALSE == data.mStyledContent->HasClass(classList->mAtom,
                                                        isCaseSensitive))) {
      result = PR_FALSE;
      break;
    }
    classList = classList->mNext;
  }

but with a MathML element, this calls |nsGenericElement::HasClass()| :

  NS_IMETHODIMP
  nsGenericElement::HasClass(nsIAtom* aClass, PRBool aCaseSensitive) const
  {
    return NS_ERROR_NOT_IMPLEMENTED;
  }

so :

(1) we need to check better the return value of HasClass() in SelectorMatches()

then how could we be using classes in MathML. MathML 1.0 specs formally
introduces class id and style elements for all mathml elements BUT there
are no mathml element classes in the content where we could override HasClass()

(2) Conclusions :

  - making class selectors work in MathML is not an easy fix at all...
  - using a DASHMATCH attribute selector like [class~="bug"] should be
    working

I'll propose a patch for (1). Reporter should use a dashmatch selector instead
of class selectors.

Comment 5

[Hixie (not reading bugmail)]

Quick question to help my understanding of the code... why is this only
affecting pseudo-elements? Also, since most languages won't support |class|, I
think that in addition to catching error return values, we should make whatever
the superclass of all the elements is implement a default hasClass that returns
false. Is that possible?

Comment 6

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Ian, from my understanding of the code, it's not affecting only pseudo-elements
:before and :after. All class selectors on an XML (not HTML) element are matching.
Making whatever the superclass of all the elements is implement a default hasClass
that returns false is not only possible, it's done but it returns 
NS_ERROR_NOT_IMPLEMENTED instead of NS_COMFALSE.

Comment 7

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: fix for SelectorMatches()

Comment 8

[Hixie (not reading bugmail)]

r=hixie on that patch, but i think we should also change nsGenericElement
(sorry, when I first read your experts I thought that that was nsMathMLElement).
If MathML wants to support classes, it should subclass nsGenericElement and
implement hasClass. Note that it would be rather good if that could be done
without duplicating the HTML version of the code...

Comment 9

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

This is why NS_COMFALSE is evil.  The method should either be
NS_IMETHOD_(PRBool) or the result should be an out param.

Comment 10

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

This would probably only affect:
 * pseudo-elements
 * selectors that are in a simple selector other than the subject
 * simple selectors that also have an ID in them
thanks to the RuleHash.

Comment 11

[rbs]

Why not eliminate NS_COMFALSE straightaway?! One advantage of bugs such as this
one is that they provide the opportunity and incentive to do a cleanup with
everybody agreable about it, and its timing.

Comment 12

[rbs]

Cleaner fix checked in via bug 8929.

Comment 13

[David A. Madore]

v