Fix wrong SVG sizes with non-integer values for viewBox width/height.

Includes a standalone reftest.
2026-09-08 08:48:39 +09:00 · 2018-09-14 20:54:19 +02:00 · 2018-09-14 20:54:19 +02:00 · ee1233bb0c
commit ee1233bb0c
parent 4f41aebb67
2 changed files with 180 additions and 4 deletions
--- a/dom/svg/test/reftest_viewport_noninteger.html
+++ b/dom/svg/test/reftest_viewport_noninteger.html
@ -0,0 +1,175 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>SVG size test</title>
+    <style>
+      body {
+        margin: 30px;
+        font-family: sans-serif;
+      }
+
+      #tests {
+        display: grid;
+        grid-template-columns: repeat(4, 1fr);
+        grid-gap: 30px;
+      }
+
+      #tests div:hover {
+        opacity: 1 !important;
+      }
+
+      #tests p {
+        text-decoration: underline dotted black;
+      }
+
+      svg {
+        background: lightgrey;
+      }
+    </style>
+  </head>
+  <body>
+    <h1>SVG size test</h1>
+
+    <p>The grey boxes below are <code>&lt;svg&gt;</code> elements.</p>
+    <p>All SVGs in each row should have the exact same size.</p>
+    <p>Each row has its own viewBox width and height: <code>viewBox="0 0 width height"</code>.</p>
+    <p>Each column has its own width/height styling. For example, <code>style="width: 200px; height: auto;"</code>.</p>
+    <p>The first column has both an explicit widht and an explicit height, so there's not much that can go wrong there. It acts as a reference.</p>
+    <p>The first row has integer viewBox width and height. Firefox then sizes all SVGs correctly.</p>
+    <p>The remaining rows have at least one non-integer viewBox width and height. Firefox then sizes the SVGs a bit wrong.</p>
+    <p>Chrome, Safari and Edge seem to pass all tests.</p>
+
+    <p id="summary"></p>
+
+    <div id="tests"></div>
+
+    <script>
+      const testsElement = document.getElementById("tests");
+      const summaryElement = document.getElementById("summary");
+
+      // Turn for instance `2.3` into `230` (px). Round to avoid floating point
+      // issues.
+      const scale = (number) => Math.round(100 * number);
+
+      const widths = [2, 2.3, 2.5, 2.8];
+      const heights = [3, 3.3, 3.5, 3.8];
+
+      let numPassed = 0;
+      let numFailed = 0;
+
+      for (const width of widths) {
+        for (const height of heights) {
+          const variations = [
+            {width, height},
+            {width: "auto", height},
+            {width, height: "auto"},
+            {width: "auto", height: "auto"},
+          ];
+
+          for (const variation of variations) {
+            const cellElement = document.createElement("div");
+
+            const titleElement = document.createElement("h2");
+            titleElement.appendChild(makeTitle(width, height, variation));
+
+            const sizeElement = document.createElement("p");
+
+            const svgWrapperElement = document.createElement("div");
+            svgWrapperElement.style.width =
+              variation.width === "auto" && variation.height === "auto"
+                ? `${scale(width)}px`
+                : "auto";
+
+            const svgElement = document.createElementNS("http://www.w3.org/2000/svg", "svg");
+            svgElement.setAttribute("viewBox", `0 0 ${width} ${height}`);
+            svgElement.style.width =
+              typeof variation.width === "number"
+                ? `${scale(variation.width)}px`
+                : variation.width;
+            svgElement.style.height =
+              typeof variation.height === "number"
+                ? `${scale(variation.height)}px`
+                : variation.height;
+
+            svgWrapperElement.appendChild(svgElement);
+
+            cellElement.appendChild(titleElement);
+            cellElement.appendChild(sizeElement);
+            cellElement.appendChild(svgWrapperElement);
+
+            testsElement.appendChild(cellElement);
+
+            const rect = svgElement.getBoundingClientRect();
+            const actual = {
+              width: Math.round(rect.width),
+              height: Math.round(rect.height),
+            };
+            const expected = {
+              width: scale(width),
+              height: scale(height),
+            };
+            const passed =
+              actual.width === expected.width &&
+              actual.height === expected.height;
+
+            const icon = passed ? "✔" : "✘";
+            const iconText = passed ? "PASS" : "FAIL";
+            const expectedText = passed ? "" : `\nExpected size: ${expected.width}x${expected.height}`;
+            sizeElement.textContent = `${icon} ${actual.width}x${actual.height}`;
+            sizeElement.title = `${iconText}. Actual size, as measured by element.getBoundingClientRect().${expectedText}`;
+            sizeElement.style.color = passed ? "lime" : "red";
+            sizeElement.style.fontWeight = passed ? "normal" : "bold";
+
+            cellElement.style.opacity = passed ? 0.5 : 1;
+
+            if (passed) {
+              numPassed++;
+            } else {
+              numFailed++;
+            }
+          }
+        }
+      }
+
+      const numTotal = numPassed + numFailed;
+      const passed = numFailed === 0;
+      const icon = passed ? "✔" : "✘";
+      summaryElement.textContent = `${icon} ${numPassed}/${numTotal} tests passed.`;
+      summaryElement.style.color = passed ? "lime" : "red";
+      summaryElement.style.fontWeight = "bold";
+
+      function makeTitle(width, height, variation) {
+        const fragment = document.createDocumentFragment();
+
+        const first = document.createElement("abbr");
+        first.textContent = `${width}/${height}`;
+        first.title = `SVG viewBox width/height: viewBox="0 0 ${width} ${height}"`;
+
+        const separator = document.createTextNode(" | ");
+
+        const second = document.createElement("abbr");
+
+        const widthString = typeof variation.width === "number" ? "px" : variation.width;
+        const heightString = typeof variation.height === "number" ? "px" : variation.height;
+        second.textContent = `${widthString}/${heightString}`;
+
+        const widthExplanation =
+          typeof variation.width === "number"
+            ? "explicit width (px)"
+            : `${variation.width} width`
+        const heightExplanation =
+          typeof variation.height === "number"
+            ? "explicit height (px)"
+            : `${variation.height} height`
+        second.title = `${widthExplanation}, ${heightExplanation}`;
+
+        fragment.appendChild(first);
+        fragment.appendChild(separator);
+        fragment.appendChild(second);
+
+        return fragment;
+      }
+    </script>
+  </body>
+</html>
--- a/layout/svg/nsSVGOuterSVGFrame.cpp
+++ b/layout/svg/nsSVGOuterSVGFrame.cpp
@ -241,8 +241,9 @@ nsSVGOuterSVGFrame::GetIntrinsicRatio()
  nsSVGLength2 &height = content->mLengthAttributes[SVGSVGElement::ATTR_HEIGHT];

  if (!width.IsPercentage() && !height.IsPercentage()) {
-    nsSize ratio(NSToCoordRoundWithClamp(width.GetAnimValue(content)),
-                 NSToCoordRoundWithClamp(height.GetAnimValue(content)));
+    nsSize ratio(
+      nsPresContext::CSSPixelsToAppUnits(width.GetAnimValue(content)),
+      nsPresContext::CSSPixelsToAppUnits(height.GetAnimValue(content)));
    if (ratio.width < 0) {
      ratio.width = 0;
    }
@ -272,8 +273,8 @@ nsSVGOuterSVGFrame::GetIntrinsicRatio()
    if (viewBoxHeight < 0.0f) {
      viewBoxHeight = 0.0f;
    }
-    return nsSize(NSToCoordRoundWithClamp(viewBoxWidth),
-                  NSToCoordRoundWithClamp(viewBoxHeight));
+    return nsSize(nsPresContext::CSSPixelsToAppUnits(viewBoxWidth),
+                  nsPresContext::CSSPixelsToAppUnits(viewBoxHeight));
  }

  return nsSVGDisplayContainerFrame::GetIntrinsicRatio();