If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

.foo:before matches all mathml elements

VERIFIED FIXED

Status

()

Core
MathML
VERIFIED FIXED
16 years ago
15 years ago

People

(Reporter: David A. Madore, Assigned: rbs)

Tracking

({css2})

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Hixie-P2], URL)

Attachments

(1 attachment)

(Reporter)

Description

16 years ago
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)
That's very... weird. It's like we're not correctly determining that mathml
elements don't match the selector or something.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: css2
Summary: bad interaction between MathML, CSS :before and class selector → .foo:before matches all mathml elements
Whiteboard: [Hixie-P2]
(Assignee)

Comment 2

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

Comment 3

16 years ago
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.
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.
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?
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.
Created attachment 85580 [details] [diff] [review]
fix for SelectorMatches()
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...
This is why NS_COMFALSE is evil.  The method should either be
NS_IMETHOD_(PRBool) or the result should be an out param.
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.
(Assignee)

Comment 11

16 years ago
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.
(Assignee)

Comment 12

15 years ago
Cleaner fix checked in via bug 8929.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
(Reporter)

Comment 13

15 years ago
v
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.