1177533 - Consider not allowing add-ons to use UA-only CSS extensions

Status: NEW

(Core :: CSS Parsing and Computation, defect)

Description

[Zack Weinberg (:zwol)]

Some of the CSS extensions that are only enabled in user-agent style sheets are unsafe in a strong sense: misuse can cause us to violate frame tree invariants.  Because of this, it was suggested in bug 1035091 (which somewhat decouples "where does this go in the cascade?" from "what CSS extensions are usable?") that add-ons should not be allowed to use UA-only CSS extensions.

Obviously this would require a careful audit of CSS poked in by add-ons before it could go forward.

Comment 1

[Masatoshi Kimura [:emk]]

Why is it specially unsafe? Add-ons can do anything danger in various ways.

Comment 2

[Cameron McCormack (:heycam)]

My initial assumption was that it could cause crashes, but after looking through all the places that we do something with the MathML properties, I think the worst thing that can happen is that we think we have a fully specified struct when we don't (or vice versa?).  This can result in incorrect style computation for an element, as we'll stop traversing up the rule tree (or keep going?) when we shouldn't.

Comment 3

[Cameron McCormack (:heycam)]

But I would want to have another look before saying for sure that it is only mildly unsafe.

Comment 4

[Zack Weinberg (:zwol)]

The comment on (the current version of) LoadSheetSync reads

   * Unsafe rules are rules that can violate key Gecko invariants if misused.
   * In particular, most anonymous box pseudoelements must be very carefully
   * styled or we will have severe problems.

That text predates the Mercurial conversion.

https://dxr.mozilla.org/mozilla-central/source/layout/style/nsCSSAnonBoxList.h has the list of anonymous box pseudoelements.  Not knowing anything more than that myself, I would be inclined to suspect whoever wrote that was thinking of XUL or tables.  Let's see if bz remembers something.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> That text predates the Mercurial conversion.

We have CVS blame via bonsai, no?

But yes, anonymous box pseudoelements refers to things like the viewport, tables, various anonymous blocks and whatnot....  I'm pretty sure that styling the viewport box display:inline used to cause crashes.

Comment 6

[Luís Miguel [:quicksaver] (not reading bugmail)]

Not knowing almost anything about how any of this happens backstage, wouldn't this be something that would be caught during manual (or even automatic?) review when submitting to AMO?