Viewport unit conversion should work in empty frame

https://bugs.webkit.org/show_bug.cgi?id=270289 rdar://116715588 Reviewed by Alan Baradlay. We hit a release assert in some cases. * LayoutTests/fast/css/viewport-unit-conversion-crash-expected.txt: Added. * LayoutTests/fast/css/viewport-unit-conversion-crash.html: Added. * Source/WebCore/css/CSSPrimitiveValue.cpp: (WebCore::CSSPrimitiveValue::convertingToLengthHasRequiredConversionData const): An empty viewport is a valid reference for resolving viewport units. The only requirement here is that we have access to one. Also test for non-fixed conversion first to make the code less confusing. Canonical link: https://commits.webkit.org/275620@main
WebKit · Mar 4, 2024 · 11d5d62 · 11d5d62
1 parent 680a3e7
commit 11d5d62
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 5 deletions.
diff --git a/LayoutTests/fast/css/viewport-unit-conversion-crash-expected.txt b/LayoutTests/fast/css/viewport-unit-conversion-crash-expected.txt
@@ -0,0 +1,2 @@
+
+This test passes if it doesn't crash
diff --git a/LayoutTests/fast/css/viewport-unit-conversion-crash.html b/LayoutTests/fast/css/viewport-unit-conversion-crash.html
@@ -0,0 +1,21 @@
+<style>
+div { vertical-align: 0vh; -webkit-user-modify: read-write }
+</style>
+<body>
+
+<div>
+<object id="target" style="contain: size" data="?"></object>
+</div>
+
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+(function() {
+	document.getSelection().collapse(document.getElementById("target"));
+	document.execCommand("underline");
+})();
+</script>
+
+<div>This test passes if it doesn't crash</div>
+
+</body>
diff --git a/Source/WebCore/css/CSSPrimitiveValue.cpp b/Source/WebCore/css/CSSPrimitiveValue.cpp
@@ -1765,18 +1765,21 @@ bool CSSPrimitiveValue::convertingToLengthHasRequiredConversionData(int lengthCo
     // return std::optional<double> instead of having this check here.
 
     bool isFixedNumberConversion = lengthConversion & (FixedIntegerConversion | FixedFloatConversion);
+    if (!isFixedNumberConversion)
+        return true;
+
     auto dependencies = computedStyleDependencies();
     if (!dependencies.rootProperties.isEmpty() && !conversionData.rootStyle())
-        return !isFixedNumberConversion;
+        return false;
 
     if (!dependencies.properties.isEmpty() && !conversionData.style())
-        return !isFixedNumberConversion;
+        return false;
 
     if (dependencies.containerDimensions && !conversionData.elementForContainerUnitResolution())
-        return !isFixedNumberConversion;
+        return false;
 
-    if (dependencies.viewportDimensions && conversionData.defaultViewportFactor().isEmpty())
-        return !isFixedNumberConversion;
+    if (dependencies.viewportDimensions && !conversionData.renderView())
+        return false;
 
     return true;
 }