Disable theming in SVG images

RESOLVED FIXED in mozilla9

Status

()

defect
RESOLVED FIXED
8 years ago
Last year

People

(Reporter: dholbert, Assigned: mats)

Tracking

Trunk
mozilla9
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

roc pointed out that it's probably possible to detect what platform a user is on (and their GTK theme, etc) by sticking a button into an SVG image.

To prevent this data leakage, we should disable theming in SVG images.
OS: Linux → All
Hardware: x86_64 → All
Posted file testcase
Mats, can you take this? We probably just need to add a "-moz-appearance:none ! important" rule to the style sheets for all SVG images.
Hmm, another thing we need to look at is whether links in SVG images get colored according to visitedness. If they do, then we're leaking visitedness information.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #3)
> Hmm, another thing we need to look at is whether links in SVG images get
> colored according to visitedness. If they do, then we're leaking visitedness
> information.

We've got bug 641731 for that.
Posted patch fix (obsolete) — Splinter Review
Assignee: nobody → matspal
Attachment #561821 - Flags: review?(roc)
The test and code does not seem to be for svg within an image tag. We're OK with SVG having themed as long as it's not <image src="something.svg/>  aren't we? That's what the standalone testcase tests - you have an image tag and the svg is within that.

I think you should have tests that svg not in an image tag is still themed.

It's a bit tricky to read the tests as they seem to be all on one line too.
It looks like this patch will apply to all SVG, which isn't what we want. We want it to only apply to the content of SVG used in an image context.
Sorry, I thought that's what you wanted...
As far as I know it's NOT possible to do this using only CSS currently.

Maybe we could introduce a :-moz-root-of-resource-document that would match
the root element of a resource document to differentiate the two cases?
A media query feature would probably be slightly better.

Whether a document is a resource document can't change dynamically so it should be quite simple to implement.

One thing that just occurred to me is that it appears an <iframe> inside an SVG image document might not set its child document to return true for IsBeingUsedAsImage ... but it should.
I can't get an <iframe> inside a SVG image to render at all.  It works when I load the
SVG directly, but when using it in a <img src=...> all I see is the <iframe>
borders, nothing inside.
Attachment #561821 - Attachment is obsolete: true
Attachment #561821 - Flags: review?(roc)
Attachment #562051 - Flags: review?(roc)
Comment on attachment 562052 [details] [diff] [review]
part 2, Use it to disable theming in SVG images

>+@media all and (-moz-is-resource-document) {
>+ foreignObject *|* {
>+   -moz-appearance: none;
>+ }
>+}

Does this prevent people from explicitly setting -moz-appearance:button? It looks like it doesn't.

Besides, CSS platform colors also still work and expose pretty much the same information, right? I don't think this is worth fighting against...
That's a good point about CSS platform colors, although I think those are already leaked through canvas drawing. I still think this is worth doing.

(In reply to Dão Gottwald [:dao] from comment #15)
> >+@media all and (-moz-is-resource-document) {
> >+ foreignObject *|* {
> >+   -moz-appearance: none;
> >+ }
> >+}
> 
> Does this prevent people from explicitly setting -moz-appearance:button? It
> looks like it doesn't.

Sigh, I missed that. It needs !important.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #16)
> That's a good point about CSS platform colors, although I think those are
> already leaked through canvas drawing. I still think this is worth doing.

You need neither canvas nor SVG for it, just CSS and getComputedStyle.
https://hg.mozilla.org/mozilla-central/rev/ec384fc33769
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla9
(In reply to Dão Gottwald [:dao] from comment #15)
> Does this prevent people from explicitly setting -moz-appearance:button? 

Good catch, thanks!
Attachment #562423 - Flags: review?(roc)
I still think this bug is pointless...
Comment on attachment 562423 [details] [diff] [review]
part 3, add the missing !important

Stealing review so it can land before the aurora merge if necessary.
Attachment #562423 - Flags: review?(roc) → review+
Depends on: 689224
You need to log in before you can comment on or make changes to this bug.