369676 - Shouldn't be possible to style scrollbars in XUL

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

It is currently possible to write selectors that will style scrollbars. However this only works in XUL documents. For example in a document stylesheet the selector

window.myCool scrollbar { ... }

will apply to all scrollbars in all <window class="myCool">s. This only works inside XUL documents, but shouldn't work anywhere.

You can use the above selector inside UA stylesheets though, which should be enough if anyone wants to create themes or apps with custom scrollbars.

Would be good to get input from the XUL guys if this is a bad idea for some reason.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch to fix

Comment 2

[Boris Zbarsky [:bzbarsky]]

Note that themes are treated as document sheets, not UA sheets....

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Really? How come pinstripe contains stylerules for styling scrollbars then?

Comment 4

[Boris Zbarsky [:bzbarsky]]

What sort of rules?

chrome://global/skin/xulscrollbars.css and chrome://global/skin/nativescrollbars.css are hardcoded in layout and applied as UA sheets to all documents.  So if that's where the skin is putting its rules they'll be fine.  If they're putting them in chrome://global/skin/, however, they won't be.

So I guess ignore comment 2.  I forgot about the special handling for the scrollbars sheets...

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Ok, so I'm requesting review for this and will land unless someone speaks up.

Comment 6

[Neil Deakin]

which scrollbars does this patch prevent from styling?

Comment 7

[Boris Zbarsky [:bzbarsky]]

The ones created via nsIAnonymousContentCreator (typically for scrollframes).  With this patch, document sheets cannot style them.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 254379 [details] [diff] [review]
Patch to fix

The change looks fine; please OK it with enn and/or neil.

Comment 9

[Neil Deakin]

In that case, this is ok.

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

It prevents document authors from styling any scrollbars. I.e. a webpage can not stick

scrollbar { background: green }

in their stylesheet. This has been disabled forever for HTML pages, but with this patch it is also disabled for XUL pages.

You can however still style scrollbars when creating a theme just as before. You could even create a theme with rules like

xul|*[scollbar-style="small"] > scrollbar { ... }

to allow different scrollbars on different elements.

So the difference with this patch is simply that the above rule only works in the theme css, not in page css.

Comment 11

[Boris Zbarsky [:bzbarsky]]

> It prevents document authors from styling any scrollbars

Not ones that are an actual <scrollbar> tag in the document, though.  You can still style those.  I think that was enn's worry.

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

What's the point of doing that?

Comment 13

[Boris Zbarsky [:bzbarsky]]

So you can build widgets with a scrollbar in them?  We have some code in the tree that does this; see http://lxr.mozilla.org/seamonkey/search?string=%253Cscrollbar

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Checked in.

Comment 15

[John Mellor (Jomel)]

Why was this changed?

This breaks some extensions (including mine) which relied on applying bindings to scrollbars in order to access them. See Bug 377292 for a bug report.

Can extensions add UA stylesheets?

Comment 16

[Martijn Wargers (dead)]

Hi John, I put a script here:
http://martijn.martijn.googlepages.com/scrpttest.htm
that perhaps can serve as an example for what you want.

Comment 17

[John Mellor (Jomel)]

Thanks Martijn, by using that code to set my stylesheet as a UA stylesheet I can apply bindings to the scrollbar again. I'm still not certain this is a useful change, but at least extensions can work around it (it still makes unprivileged XUL pages unable to script their overflow:auto scrollbars without wrapping them in a scrollbox though).

Comment 18

[Boris Zbarsky [:bzbarsky]]

> Why was this changed?

Because it was a rather pointless inconsistency between XUL and non-XUL documents and had security and code complexity implications that were a bit troubling.