Use image documents to render toplevel, non-opener SVG documents behind a pref
Categories
(Core :: Layout, enhancement)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox110 | --- | affected |
People
(Reporter: Gijs, Assigned: Gijs)
References
Details
Attachments
(3 files)
Screenshot of PoC
(I assumed this was already on file somewhere but couldn't see anything.)
This would basically eliminate the attack vector from bug 1753004, and also has better ergonomics for day-to-day use (ImageDocument will center the images instead of sticking them in the top left, give me better and convenient zoom controls, and the SVG size in the tab title). This would improve "open image in a new tab" and files opened from the downloads popup, but wouldn't touch SVG files with scriptable access from an existing website.
Hypothetically, if someoned served https://example.com/ or similar toplevel sites as an SVG and hoped they could get document rendering, that would break with the pref toggled.
If I can figure out how to fire some kind of event at frontend from the depths of ImageDocument, we could potentially show UI to the user if we detect script tags in the SVG, to allow them to opt in to the document style rendering (with an additional load flag on the channel that would bypass the use of ImageDocument).
|
Assignee | |
Comment 1•2 years ago
|
||
|
|
Assignee | |
Comment 2•2 years ago
|
||
Depends on D165781
|
|
Assignee | |
Comment 3•2 years ago
|
||
Open to feedback about this UI; I haven't fully finished implementing it (wondering e.g. if we're going to need to use permissions here so people can enable/disable script permanently on a per-domain basis or w/e, and if we should be using the permission doorhangers instead of the notification bar - but that feels more (too?) intrusive.
I'd be tempted to suggest landing something like this with the pref turned on for Nightly only, with telemetry for how often the notification bar shows up, to see how often people see this on Nightly (which seems like a decent upper bound for how often people are likely to see it on release). Perhaps x-ref bug 1745835...
|
||
Comment 4•2 years ago
•
|
||
As noted in phab (not sure whether the discussion is better to have here or there), scripts are only one of several things that might prompt someone to want the full-SVG-document experience. Not sure whether it's practical to try to catch all of them and offer a prompt if any these features are present; but it's worth considering.
Also: I tried building with these patches and toggling the pref, and (under that build/configuration) I noticed two papercuts regarding our ImageDocument presentation of SVG-documents-that-lack-a-height-and-width, like this one:
data:image/svg+xml,
<svg xmlns="http://www.w3.org/2000/svg" preserveAspectRatio="xMidYMid meet" viewBox="0 0 16 16">
<circle cx="8" cy="8" r="8" fill="lime"/></svg>
[edit: fixed to decapitalize "meet" as Robert noted]
ISSUE #1: When you view this as an image document (with the patch applied and the pref flipped), you can't zoom in or out; the image remains the same size, regardless of what you do with our full-page-zoom UI. We show the magnifying glass as if you're zooming, but nothing seems to change. That's kinda unexpected (but it might make some amount of sense).
ISSUE #2: Our image document seems like it's ignoring the "meet|slice" parameter for the SVG preserveAspectRatio attribute (which essentially does contain-vs-cover scaling), and it ends up feeling like it produces a weird intermediate between the two regardless of what you choose. This behavior falls out of our ImageDocument CSS, which produces odd results for the above SVG image. Here's a simplified version of what the image document ends up being for the above SVG file (you can view this in regular Firefox to get an idea of how it'll look):
data:text/html,<!DOCTYPE html>
<img style="position: absolute; inset: 0; margin: auto" src="data:image/svg+xml,
<svg xmlns='http://www.w3.org/2000/svg' preserveAspectRatio='xMidYMid meet' viewBox='0 0 16 16'>
<circle cx='8' cy='8' r='8' fill='lime'/></svg>
">
If you view this^ in a wide rectangular window, then the top part gets clipped, but the bottom part is scrollable (due to how our image document's position:absolute;inset:0;margin:auto; CSS resolves for this image). This is not an ideal presentation of this image document, and it's substantially different from how it looks as a regular "full" SVG document.
So: if we do end up taking action on this bug, I think we need to tread carefully around making sure that we don't end up breaking the presentation of such SVG documents. This will probably require adding more subtlety to our ImageDocument CSS and/or C++.
|
||
Comment 5•2 years ago
|
||
SVg is case sensitive so you need to use meet rather than Meet
|
|
||
Comment 6•2 years ago
•
|
||
[writing some more notes & midair'd with Robert -- thanks for the correction Robert! I edited the comment to fix that. That didn't end up meaningfully affecting the outcome, though, I think because meet is the default. :) So my notes there still stand. The rest of this comment is further thoughts on my "Issue #2".]
Note that strictly speaking, our rendering in issue #2 is "correct" given the actual image-document markup that we're rendering (with our image document CSS). The issue is that our image-document-CSS isn't designed for an image that has an aspect ratio but lacks an intrinsic width and height, and it produces a not-super-useful result when applied to such an image.
If we add an explicit width and height to the img to make it fill the viewport, then we get a better rendering and can differentiate between meet and slice. Here's a mockup of that (having added width:100vw; height: 100vh;)
With meet:
data:text/html,<!DOCTYPE html>
<img style="position: absolute; inset: 0; width:100vw; height: 100vh; margin: auto" src="data:image/svg+xml,
<svg xmlns='http://www.w3.org/2000/svg' preserveAspectRatio='xMidYMid meet' viewBox='0 0 16 16'>
<circle cx='8' cy='8' r='8' fill='lime'/></svg>
">
vs. with slice:
data:text/html,<!DOCTYPE html>
<img style="position: absolute; inset: 0; width:100vw; height: 100vh; margin: auto" src="data:image/svg+xml,
<svg xmlns='http://www.w3.org/2000/svg' preserveAspectRatio='xMidYMid slice' viewBox='0 0 16 16'>
<circle cx='8' cy='8' r='8' fill='lime'/></svg>
">
Both of these^ render the same as their actual "full" SVG document, so they represents kind of the ideal of what we'd like our image document to look like. But obviously we don't want to unconditionally add width:100vw; height: 100vh to our image documents (since we want to use the intrinsic size if there is one); so this isn't really a general solution. Maybe we need to add some logic to detect the presence/absence of intrinsic sizes, at some point in our image-document preparation code.
|
||
Comment 7•2 years ago
•
|
||
I haven't thought about this deeply yet, but the main question I have is what is the rational for treating HTML with script differently to SVG with script?
I'd also note this isn't compatible with Chrome which happily runs the script (the blue circle is draggable) in https://jwatt.org/svg/canvas-svg.svg whether loaded with https:// or file://.
|
|
Assignee | |
Comment 8•2 years ago
|
||
(In reply to Jonathan Watt [:jwatt] from comment #7)
I haven't thought about this deeply yet, but the main question I have is what is the rational for treating HTML with script differently to SVG with script?
My motivation for looking at this has been twofold.
First of all, as someone who primarily uses SVG images without script or interactive elements, I think displaying in an image document should be the default for those files - centered in the middle of the content area, with ideally the same controls/options as other image document formats. From a user perspective, it's odd that when you "open image in new tab", for png/jpeg/webp/etc., the result is radically different from the case when the image is an svg file - you can't actually tell from the UI that that's the case until after the tab opens.
Second, from a security PoV, websites as well as Firefox have had repeated issues that come with users/developers generally wanting to treat SVG files as images, but being burned by them containing script. HTML doesn't have that same issue because nobody thinks of HTML as an image format. Previous discussions around this happened in (among other places...) bug 1753004, bug 1018790, and in standards tickets like https://github.com/w3c/svgwg/issues/266 , https://github.com/w3c/svgwg/issues/837 .
In bug 1753004 comment 3, Daniel suggested we may be able to consider doing this if there's some way for users to switch between the two modes, which is what I tried to demo with the PoC patches.
I'm happy to have a conversation around what the defaults should be for web/file URLs or other heuristics - mostly I'm trying to make some forward progress on the issue by showing what is/isn't possible easily. It may be that we'd need to pursue bug 1745835 and telemetry for this first - I'd defer to the relevant module folks for that. :-)
Description
•