Closed
Bug 1338042
Opened 7 years ago
Closed 7 years ago
SVG graphic in background-image does not preserve aspect ratio when height is not specified on SVG
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: birtles, Assigned: mantaroh)
References
()
Details
(Whiteboard: [webcompat])
Attachments
(2 files)
I'm really not sure what is going on here, but for the test URL you can see that the Mario in the middle appears squashed. In Chrome and Edge, however, he appears normal. I've attached a reduced test case that shows the same problem with an arrow graphic. Note that the SVG graphic specifies a width but not a height. If I add a height it appears as expected (not squashed). It seems the aspect ratio could be inferred from the viewBox. I'm not sure if we're supposed to do that or not, it's possible we're even doing the correct thing, but at least Chrome and Edge do. It's also likely this is a dupe of an existing bug but I'm filing this for now until I find that bug.
Reporter | ||
Comment 1•7 years ago
|
||
Mantaroh, here is another SVG-related bug that would be great to fix if you have time. This sort of interop bug is the sort thing that frustrates authors a lot. I suspect this cross a number of spec boundaries, however, and will take some time to work out the correct sizing behavior.
Updated•7 years ago
|
Whiteboard: [webcompat]
Reporter | ||
Comment 2•7 years ago
|
||
Safari appears to matches the behavior of Blink and Edge for this test case.
Comment 3•7 years ago
|
||
Display of images where attributes are missing is pretty much unspecced. All UAs try to guess the user's actual intent. The accuracy of this guessing varies.
Reporter | ||
Comment 4•7 years ago
|
||
CSS images 3 does talk about an intrinsic dimensions in a way that suggests what is expected but without actually defining how this works for SVG: "The term intrinsic dimensions refers to the set of the intrinsic height, intrinsic width, and intrinsic aspect ratio (the ratio between the width and height), each of which may or may not exist for a given object. These intrinsic dimensions represent a preferred or natural size of the object itself; that is, they are not a function of the context in which the object is used. CSS does not define how the intrinsic dimensions are found in general. "Raster images are an example of an object with all three intrinsic dimensions. SVG images designed to scale might have only an intrinsic aspect ratio; SVG images can also be created with only an intrinsic width or height. CSS gradients, defined in this specification, are an example of an object with no intrinsic dimensions at all. Another example of this is embedded documents, such as the <iframe> element in HTML. An object cannot have only two intrinsic dimensions, as any two automatically define the third." (https://drafts.csswg.org/css-images-3/#intrinsic-dimensions) So in this case we have an intrinsic width and an intrinsic aspect ratio. As the spec text if we have two intrinsic dimensions we can calculate the third. So presumably we should treat this image as having an intrinsic height calculated from the aspect ratio and width. That said, I'm not sure if that's actually where we're coming unstuck. It may be we calculate correct intrinsic height but when we size the graphic to the box we do it differently. And I agree it doesn't actually spell out the mapping to SVG for these properties.
Assignee | ||
Comment 5•7 years ago
|
||
In the gecko, we will calculate and set image size when drawing the image.[1] [1] https://dxr.mozilla.org/mozilla-central/rev/698de2db1b16a5ef3c6a39f0f72885e69aee4022/layout/painting/nsCSSRendering.cpp#4048-4052 This size is based on the value from imgIContainer object. (in this case, VectorImage which is subclass) If we don't set the hight value only, we will use the default size.[2] [2] https://dxr.mozilla.org/mozilla-central/rev/698de2db1b16a5ef3c6a39f0f72885e69aee4022/layout/painting/nsCSSRendering.cpp#5585-5594 I think that we will need to calculate the width or height value from two intrinsic dimensions. Rough implement: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8fc1f60966cfb05c71e21197ff1af5e1d57189ba
Assignee: nobody → mantaroh
Assignee | ||
Comment 6•7 years ago
|
||
Add the reftests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1067269802a1b95b79b4a7524a650e7da7defed1
Status: NEW → ASSIGNED
Assignee | ||
Comment 7•7 years ago
|
||
Fix typo and removed unnecessary tests. https://treeherder.mozilla.org/#/jobs?repo=try&revision=c2f7376387cb3cda96d961e27e382b19510f73ee
Comment hidden (mozreview-request) |
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8840299 [details] Bug 1338042 - Compute a missing width/height from the aspect ratio and the other dimension. https://reviewboard.mozilla.org/r/114740/#review116286 r+ with review comments addressed ::: layout/painting/nsCSSRendering.cpp:5428 (Diff revision 1) > if (haveHeight) { > result.SetHeight(nsPresContext::CSSPixelsToAppUnits(imageIntSize.height)); > } > + > + // If we can calculate width or height from two intrinsic factors, > + // we will compute third intrinsic dimension. The comment is perhaps better written as // If we know the aspect ratio and one of the dimensions, // we can compute the other missing width or height. ::: layout/reftests/svg/reftest.list:478 (Diff revision 1) > == mask-use-element-01.svg pass.svg > == clip-use-element-01.svg pass.svg > == clip-use-element-02.svg pass.svg > == filter-use-element-01.svg pass.svg > + > +== background-svg-without-height.html background-ref.html reftests are both generally alphabetically ordered in this file (except for this mess at the end that needs fixing), and they are also numbered in case we want to create multiple tests for the same issue.
Attachment #8840299 -
Flags: review?(longsonr) → review+
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8840299 [details] Bug 1338042 - Compute a missing width/height from the aspect ratio and the other dimension. https://reviewboard.mozilla.org/r/114740/#review116292 also the checkin comment is misspelled it should be Compute a missing width/height from the aspect ratio and the other dimension.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 12•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8840299 [details] Bug 1338042 - Compute a missing width/height from the aspect ratio and the other dimension. https://reviewboard.mozilla.org/r/114740/#review116292 Thanks, Robert. I addressed comments and tests orders.
Comment 13•7 years ago
|
||
Pushed by mantaroh@gmail.com: https://hg.mozilla.org/integration/autoland/rev/f95acc1bb74e Compute a missing width/height from the aspect ratio and the other dimension. r=longsonr+218550
Comment 14•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f95acc1bb74e
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•