305026 - [FIXr]Document how calls on nsIStyleSheetService interact with userContent.css

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[Simon Fraser [no longer active]]

When loading stylesheets (e.g. for ad blocking) via nsIStyleSheetService, it's
unclear how they will interact with userContent.css. Specifically, it's unclear
whether rules in the dynamically-loaded stylesheet will override those in
userContent.css, or whether it depends on which was loaded last.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Documentation

The nsStyleSet.h change just makes the docs match the code change made in bug
252578

Comment 2

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

Comment on attachment 193183 [details] [diff] [review]
Documentation

r+sr=dbaron, except I think the comment that you're changing in nsStyleSet.h
was actually a comment describing the ordering of the arrays within
mRuleProcessors, which was *removed* in bug 252578.  I think the comment should
say "Levels of the cascade, from least significant to most".  The use of "All"
seems incorrect.

Comment 3

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

...except that wording on significance is backward for !important, so I'd say:

// The "origins" of the CSS cascade, from lowest precedence to highest (for
// non-!important rules).

Comment 4

[Simon Fraser [no longer active]]

If I'm reading this right, rules in userContent.css will be clobbered by rules
in sheets loaded via nsIStyleSheetService (is this true for !important rules
too?). This is unfortunate, since it prevents the user from overriding rules in
an application-provided stylesheet with rules in their user-editable stylesheet
(i.e. the user no longer has ultimate control).

Comment 5

[Boris Zbarsky [:bzbarsky]]

> (is this true for !important rules too?).

!important rules in userContent.css will be overridden by !important rules
loaded via the stylesheet service if the selectors ahve the same specificity.

I see no problem with putting userContent.css after the nsIStyleSheetService
sheets for the user level (though I still think that they should come after
ua.css for the UA leve).  David, Brian, what do you think?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: With those changes

This addresses the comments, and I changed the relative ordering of
userChrome/Content.css and the sheets added via this API; I think Simon makes a
good argument for having userChrome/Content.css be last.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Oh, I'm going to add another sentence or two saying that relative ordering of
sheets added via this API is undefined.

Comment 8

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

Comment on attachment 193209 [details] [diff] [review]
With those changes

>-    sheetService->UserStyleSheets()->EnumerateForwards(AppendUserSheet,
>+    sheetService->UserStyleSheets()->EnumerateForwards(PrependUserSheet,

why not make this EnumerateBackwards and not have the relative ordering be
undefined?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Because then I'd have to document that the relative ordering is dependent on the
order of sheet registration and I'm not really happy doing that.  Especially
because right now that's only true because of the data storage we're using
internally (an array).

Comment 10

[Simon Fraser [no longer active]]

I don't suppose this would get onto the 1.8 branch, would it? Camino is using
nsIStyleSheetService for ad blocking, and it would be cool if they could disable
certain ad blocking rules via userContent.css.

Comment 11

[Boris Zbarsky [:bzbarsky]]

If that gets reviewed, I plan to try and land it on branch...

Comment 12

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

Comment on attachment 193209 [details] [diff] [review]
With those changes

OK, but could you change it to EnumerateBackwards anyway?  Somebody might
accidentally start depending on current behavior, and if so, I'd at least like
to have them depend on something that's consistent.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated to comments

This is very safe and makes this API easier to use (and documents it better).

Comment 14

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Fixed on branch.