LibWeb/SVG: Parse comma-separated SVG viewBox

From the SVG spec

The value of the ‘viewBox’ attribute is a list of four numbers <min-x>,
<min-y>, <width> and <height>, separated by whitespace and/or a comma...

Currently try_parse_view_box will fail to parse the attribute if the
values are separated by commas.

This change replaces try_parse_view_box with a more correct
implementation. It will reside in the AttributeParser.cpp. This new
implementation correctly handles comma-separated viewBox values, and is
also more robust against invalid inputs.

Additionally, it adds a new test case to ensure viewBox values with
various syntax are parsed correctly and invalid values are rejected.

Libraries/LibWeb/CMakeLists.txt

@@ -921,7 +921,6 @@ set(SOURCES
     SVG/SVGUseElement.cpp
     SVG/SVGViewElement.cpp
     SVG/TagNames.cpp
-    SVG/ViewBox.cpp
     TrustedTypes/TrustedHTML.cpp
     TrustedTypes/TrustedScript.cpp
     TrustedTypes/TrustedScriptURL.cpp

Libraries/LibWeb/SVG/AttributeParser.cpp

@@ -702,6 +702,48 @@ Optional<Vector<Transform>> AttributeParser::parse_transform()
     return transform_list;
 }
 
+Optional<ViewBox> AttributeParser::parse_viewbox(StringView input)
+{
+    AttributeParser parser { input };
+    ViewBox viewbox;
+
+    parser.parse_whitespace();
+    auto maybe_min_x = parser.parse_coordinate();
+    if (maybe_min_x.is_error())
+        return {};
+    viewbox.min_x = maybe_min_x.value();
+
+    if (!parser.match_comma_whitespace())
+        return {};
+    parser.parse_comma_whitespace();
+    auto maybe_min_y = parser.parse_coordinate();
+    if (maybe_min_y.is_error())
+        return {};
+    viewbox.min_y = maybe_min_y.value();
+
+    if (!parser.match_comma_whitespace())
+        return {};
+    parser.parse_comma_whitespace();
+    auto maybe_width = parser.parse_length();
+    if (maybe_width.is_error())
+        return {};
+    viewbox.width = maybe_width.value();
+
+    if (!parser.match_comma_whitespace())
+        return {};
+    parser.parse_comma_whitespace();
+    auto maybe_height = parser.parse_length();
+    if (maybe_height.is_error())
+        return {};
+    viewbox.height = maybe_height.value();
+
+    parser.parse_whitespace();
+    if (!parser.done())
+        return {};
+
+    return viewbox;
+}
+
 bool AttributeParser::match_whitespace() const
 {
     if (done())

Libraries/LibWeb/SVG/AttributeParser.h

@@ -76,6 +76,13 @@ enum class SVGUnits {
     UserSpaceOnUse
 };
 
+struct ViewBox {
+    double min_x { 0 };
+    double min_y { 0 };
+    double width { 0 };
+    double height { 0 };
+};
+
 using GradientUnits = SVGUnits;
 using MaskUnits = SVGUnits;
 using MaskContentUnits = SVGUnits;
@@ -141,6 +148,7 @@ public:
     static Optional<PreserveAspectRatio> parse_preserve_aspect_ratio(StringView input);
     static Optional<SVGUnits> parse_units(StringView input);
     static Optional<SpreadMethod> parse_spread_method(StringView input);
+    static Optional<ViewBox> parse_viewbox(StringView input);
 
 private:
     AttributeParser(StringView source);

Libraries/LibWeb/SVG/SVGFitToViewBox.cpp

@@ -28,7 +28,7 @@ void SVGFitToViewBox::attribute_changed(DOM::Element& element, FlyString const&
         if (!value.has_value()) {
             m_view_box_for_bindings->set_nulled(true);
         } else {
-            m_view_box = try_parse_view_box(value.value_or(String {}));
+            m_view_box = AttributeParser::parse_viewbox(value.value_or(String {}));
             m_view_box_for_bindings->set_nulled(!m_view_box.has_value());
             if (m_view_box.has_value()) {
                 m_view_box_for_bindings->set_base_val(Gfx::DoubleRect { m_view_box->min_x, m_view_box->min_y, m_view_box->width, m_view_box->height });

Libraries/LibWeb/SVG/SVGFitToViewBox.h

@@ -9,7 +9,6 @@
 #include <LibJS/Heap/Cell.h>
 #include <LibWeb/SVG/AttributeParser.h>
 #include <LibWeb/SVG/SVGAnimatedString.h>
-#include <LibWeb/SVG/ViewBox.h>
 
 namespace Web::SVG {
 

Libraries/LibWeb/SVG/SVGGraphicsElement.h

@@ -17,7 +17,6 @@
 #include <LibWeb/SVG/SVGFitToViewBox.h>
 #include <LibWeb/SVG/SVGGradientElement.h>
 #include <LibWeb/SVG/TagNames.h>
-#include <LibWeb/SVG/ViewBox.h>
 
 namespace Web::SVG {
 

Libraries/LibWeb/SVG/SVGSVGElement.h

@@ -15,7 +15,6 @@
 #include <LibWeb/SVG/SVGGraphicsElement.h>
 #include <LibWeb/SVG/SVGLength.h>
 #include <LibWeb/SVG/SVGTransform.h>
-#include <LibWeb/SVG/ViewBox.h>
 #include <LibWeb/WebIDL/Types.h>
 
 namespace Web::SVG {

Libraries/LibWeb/SVG/ViewBox.cpp

@@ -1,62 +0,0 @@
-/*
- * Copyright (c) 2021, Andreas Kling <andreas@ladybird.org>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#include <AK/CharacterTypes.h>
-#include <AK/GenericLexer.h>
-#include <AK/Optional.h>
-#include <AK/StringView.h>
-#include <LibWeb/SVG/ViewBox.h>
-
-namespace Web::SVG {
-
-Optional<ViewBox> try_parse_view_box(StringView string)
-{
-    // FIXME: This should handle all valid viewBox values.
-
-    GenericLexer lexer(string);
-
-    enum State {
-        MinX,
-        MinY,
-        Width,
-        Height,
-    };
-    int state { State::MinX };
-    ViewBox view_box;
-
-    while (!lexer.is_eof()) {
-        lexer.consume_while([](auto ch) { return is_ascii_space(ch); });
-        auto token = lexer.consume_until([](auto ch) { return is_ascii_space(ch) && ch != ','; });
-        auto maybe_number = token.to_number<float>();
-        if (!maybe_number.has_value())
-            return {};
-        switch (state) {
-        case State::MinX:
-            view_box.min_x = maybe_number.value();
-            break;
-        case State::MinY:
-            view_box.min_y = maybe_number.value();
-            break;
-        case State::Width:
-            if (*maybe_number < 0)
-                return {};
-            view_box.width = maybe_number.value();
-            break;
-        case State::Height:
-            if (*maybe_number < 0)
-                return {};
-            view_box.height = maybe_number.value();
-            break;
-        default:
-            return {};
-        }
-        state += 1;
-    }
-
-    return view_box;
-}
-
-}

Libraries/LibWeb/SVG/ViewBox.h

@@ -1,23 +0,0 @@
-/*
- * Copyright (c) 2021, Andreas Kling <andreas@ladybird.org>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#pragma once
-
-#include <AK/Forward.h>
-#include <LibWeb/Forward.h>
-
-namespace Web::SVG {
-
-struct ViewBox {
-    double min_x { 0 };
-    double min_y { 0 };
-    double width { 0 };
-    double height { 0 };
-};
-
-Optional<ViewBox> try_parse_view_box(StringView);
-
-}

Meta/gn/secondary/Userland/Libraries/LibWeb/SVG/BUILD.gn

@@ -48,6 +48,5 @@ source_set("SVG") {
     "SVGTransformList.cpp",
     "SVGUseElement.cpp",
     "TagNames.cpp",
-    "ViewBox.cpp",
   ]
 }

Tests/LibWeb/Text/expected/SVG/svg-viewbox-syntax.txt

@@ -0,0 +1,18 @@
+0
+1
+2
+3
+4
+5
+6
+7
+8
+9
+10
+11
+null
+null
+null
+null
+null
+null

Tests/LibWeb/Text/input/SVG/svg-viewbox-syntax.html

@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<script src="../include.js"></script>
+<svg xmlns="http://www.w3.org/2000/svg" style="display: none" id="svg-element"></svg>
+<script>
+    test(() => {
+        const svgElement = document.getElementById("svg-element");
+        svgElement.setAttribute("viewBox", "0,1,2,3");
+        println(svgElement.viewBox.baseVal.x);
+        println(svgElement.viewBox.baseVal.y);
+        println(svgElement.viewBox.baseVal.width);
+        println(svgElement.viewBox.baseVal.height);
+        svgElement.setAttribute("viewBox", "  4  ,  5  ,  6  ,  7  ");
+        println(svgElement.viewBox.baseVal.x);
+        println(svgElement.viewBox.baseVal.y);
+        println(svgElement.viewBox.baseVal.width);
+        println(svgElement.viewBox.baseVal.height);
+        svgElement.setAttribute("viewBox", "8 9,10 11");
+        println(svgElement.viewBox.baseVal.x);
+        println(svgElement.viewBox.baseVal.y);
+        println(svgElement.viewBox.baseVal.width);
+        println(svgElement.viewBox.baseVal.height);
+        svgElement.setAttribute("viewBox", "");
+        println(svgElement.viewBox.baseVal);
+        svgElement.setAttribute("viewBox", " , , , ");
+        println(svgElement.viewBox.baseVal);
+        svgElement.setAttribute("viewBox", "12");
+        println(svgElement.viewBox.baseVal);
+        svgElement.setAttribute("viewBox", "13,");
+        println(svgElement.viewBox.baseVal);
+        svgElement.setAttribute("viewBox", ",14");
+        println(svgElement.viewBox.baseVal);
+        svgElement.setAttribute("viewBox", "15,16,17,18,");
+        println(svgElement.viewBox.baseVal);
+    });
+</script>