Use object for Top / Left / Bottom / Right in MarginOptions (#2788)

campersau · kblok · web-flow · commit 71ff8222f331 · 2024-10-18T13:13:17.000-04:00
* Use object for Top / Left / Bottom / Right in MarginOptions
This is similar to the object Width / Height in PdfOptions and all of them already got converted by ConvertPrintParameterToInches

* Add back PdfOptionsShouldWorkWithMarginWithNoUnits test

* Expand comment for PrimitiveTypeConverter

* Turn test into unit test

* bump System.Text.Json

* chrome 129.0.6668.100

---------

Co-authored-by: Darío Kondratiuk &lt;dariokondratiuk@gmail.com&gt;
diff --git a/README.md b/README.md
@@ -90,7 +90,7 @@ await page.GoToAsync("http://www.google.com"); // In case of fonts being loaded
 await page.EvaluateExpressionHandleAsync("document.fonts.ready"); // Wait for fonts to be loaded. Omitting this might result in no text rendered in pdf.
 await page.PdfAsync(outputFile);
 ```
-<sup><a href='https://github.com/hardkoded/puppeteer-sharp/blob/master/lib/PuppeteerSharp.Tests/PageTests/PdfTests.cs#L23-L33' title='Snippet source file'>snippet source</a> | <a href='#snippet-pdfasync_example' title='Start of snippet'>anchor</a></sup>
+<sup><a href='https://github.com/hardkoded/puppeteer-sharp/blob/master/lib/PuppeteerSharp.Tests/PageTests/PdfTests.cs#L24-L34' title='Snippet source file'>snippet source</a> | <a href='#snippet-pdfasync_example' title='Start of snippet'>anchor</a></sup>
 <!-- endSnippet -->
 
 ### Inject HTML
diff --git a/lib/PuppeteerSharp.Tests/PageTests/PdfTests.cs b/lib/PuppeteerSharp.Tests/PageTests/PdfTests.cs
@@ -3,6 +3,7 @@
 using System.Text.Json;
 using System.Threading.Tasks;
 using NUnit.Framework;
+using PuppeteerSharp.Cdp;
 using PuppeteerSharp.Media;
 using PuppeteerSharp.Nunit;
 
@@ -127,6 +128,8 @@ public void PdfOptionsShouldBeSerializable()
         {
             var pdfOptions = new PdfOptions
             {
+                Width = 100,
+                Height = 100,
                 Format = PaperFormat.A4,
                 DisplayHeaderFooter = true,
                 MarginOptions = new MarginOptions
@@ -145,25 +148,13 @@ public void PdfOptionsShouldBeSerializable()
         }
 
         [Test]
-        public void PdfOptionsShouldWorkWithMarginWithNoUnits()
+        public void ConvertPrintParameterToInchesTests()
         {
-            var pdfOptions = new PdfOptions
-            {
-                Format = PaperFormat.A4,
-                DisplayHeaderFooter = true,
-                MarginOptions = new MarginOptions
-                {
-                    Top = "0",
-                    Right = "0",
-                    Bottom = "0",
-                    Left = "0"
-                },
-                FooterTemplate = "<div id=\"footer-template\" style=\"font-size:10px !important; color:#808080; padding-left:10px\">- <span class=\"pageNumber\"></span> - </div>"
-            };
-
-            var serialized = JsonSerializer.Serialize(pdfOptions);
-            var newPdfOptions = JsonSerializer.Deserialize<PdfOptions>(serialized);
-            Assert.That(newPdfOptions, Is.EqualTo(pdfOptions));
+            Assert.That(CdpPage.ConvertPrintParameterToInches("10"), Is.EqualTo(10m / 96));
+            Assert.That(CdpPage.ConvertPrintParameterToInches("10px"), Is.EqualTo(10m / 96));
+            Assert.That(CdpPage.ConvertPrintParameterToInches("0"), Is.EqualTo(0));
+            Assert.That(CdpPage.ConvertPrintParameterToInches("0px"), Is.EqualTo(0));
+            Assert.That(CdpPage.ConvertPrintParameterToInches("10in"), Is.EqualTo(10));
         }
     }
 }
diff --git a/lib/PuppeteerSharp/BrowserData/Chrome.cs b/lib/PuppeteerSharp/BrowserData/Chrome.cs
@@ -13,7 +13,7 @@ public static class Chrome
         /// <summary>
         /// Default chrome build.
         /// </summary>
-        public static string DefaultBuildId => "128.0.6613.119";
+        public static string DefaultBuildId => "129.0.6668.100";
 
         internal static async Task<string> ResolveBuildIdAsync(ChromeReleaseChannel channel)
             => (await GetLastKnownGoodReleaseForChannel(channel).ConfigureAwait(false)).Version;
diff --git a/lib/PuppeteerSharp/Cdp/CdpPage.cs b/lib/PuppeteerSharp/Cdp/CdpPage.cs
@@ -731,6 +731,48 @@ internal static async Task<Page> CreateAsync(
         }
     }
 
+    internal static decimal ConvertPrintParameterToInches(object parameter)
+    {
+        if (parameter == null)
+        {
+            return 0;
+        }
+
+        decimal pixels;
+        if (parameter is decimal or int)
+        {
+            pixels = Convert.ToDecimal(parameter, CultureInfo.CurrentCulture);
+        }
+        else
+        {
+            var text = parameter.ToString();
+            var unit = text.Length > 2 ? text.Substring(text.Length - 2).ToLower(CultureInfo.CurrentCulture) : string.Empty;
+            string valueText;
+            if (GetPixels(unit) is { })
+            {
+                valueText = text.Substring(0, text.Length - 2);
+            }
+            else
+            {
+                // In case of unknown unit try to parse the whole parameter as number of pixels.
+                // This is consistent with phantom's paperSize behavior.
+                unit = "px";
+                valueText = text;
+            }
+
+            if (decimal.TryParse(valueText, NumberStyles.Any, CultureInfo.InvariantCulture.NumberFormat, out var number))
+            {
+                pixels = number * GetPixels(unit).Value;
+            }
+            else
+            {
+                throw new ArgumentException($"Failed to parse parameter value: '{text}'", nameof(parameter));
+            }
+        }
+
+        return pixels / 96;
+    }
+
     /// <inheritdoc />
     protected override async Task ExposeFunctionAsync(string name, Delegate puppeteerFunction)
     {
@@ -1214,48 +1256,6 @@ await PrimaryTargetClient.SendAsync(
         }
     }
 
-    private decimal ConvertPrintParameterToInches(object parameter)
-    {
-        if (parameter == null)
-        {
-            return 0;
-        }
-
-        decimal pixels;
-        if (parameter is decimal or int)
-        {
-            pixels = Convert.ToDecimal(parameter, CultureInfo.CurrentCulture);
-        }
-        else
-        {
-            var text = parameter.ToString();
-            var unit = text.Length > 2 ? text.Substring(text.Length - 2).ToLower(CultureInfo.CurrentCulture) : string.Empty;
-            string valueText;
-            if (GetPixels(unit) is { })
-            {
-                valueText = text.Substring(0, text.Length - 2);
-            }
-            else
-            {
-                // In case of unknown unit try to parse the whole parameter as number of pixels.
-                // This is consistent with phantom's paperSize behavior.
-                unit = "px";
-                valueText = text;
-            }
-
-            if (decimal.TryParse(valueText, NumberStyles.Any, CultureInfo.InvariantCulture.NumberFormat, out var number))
-            {
-                pixels = number * GetPixels(unit).Value;
-            }
-            else
-            {
-                throw new ArgumentException($"Failed to parse parameter value: '{text}'", nameof(parameter));
-            }
-        }
-
-        return pixels / 96;
-    }
-
     private Clip GetIntersectionRect(Clip clip, BoundingBox viewport)
     {
         var x = Math.Max(clip.X, viewport.X);
diff --git a/lib/PuppeteerSharp/Helpers/Json/PrimitiveTypeConverter.cs b/lib/PuppeteerSharp/Helpers/Json/PrimitiveTypeConverter.cs
@@ -0,0 +1,65 @@
+#nullable enable
+
+using System;
+using System.Text.Json;
+using System.Text.Json.Serialization;
+
+namespace PuppeteerSharp.Helpers.Json
+{
+    /// <summary>
+    /// Support types (<see cref="decimal"/>, <see cref="int"/> and <see cref="string"/>)
+    /// used by <see cref="PdfOptions"/> and <see cref="Media.MarginOptions"/> for serialization / deserialization.
+    /// For usecases like <see href="https://github.com/hardkoded/puppeteer-sharp/issues/1001"/>.
+    /// </summary>
+    internal sealed class PrimitiveTypeConverter : JsonConverter<object>
+    {
+        public override object? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+        {
+            if (reader.TokenType == JsonTokenType.Null)
+            {
+                return null;
+            }
+            else if (reader.TokenType == JsonTokenType.String)
+            {
+                return reader.GetString();
+            }
+            else if (reader.TokenType == JsonTokenType.Number)
+            {
+                if (reader.TryGetInt32(out var i))
+                {
+                    return i;
+                }
+                else if (reader.TryGetDecimal(out var dec))
+                {
+                    return dec;
+                }
+            }
+
+            return JsonSerializer.Deserialize(ref reader, typeToConvert, options);
+        }
+
+        public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options)
+        {
+            if (value is null)
+            {
+                writer.WriteNullValue();
+            }
+            else if (value is string str)
+            {
+                writer.WriteStringValue(str);
+            }
+            else if (value is decimal dec)
+            {
+                writer.WriteNumberValue(dec);
+            }
+            else if (value is int i)
+            {
+                writer.WriteNumberValue(i);
+            }
+            else
+            {
+                JsonSerializer.Serialize(writer, value, options);
+            }
+        }
+    }
+}
diff --git a/lib/PuppeteerSharp/Media/MarginOptions.cs b/lib/PuppeteerSharp/Media/MarginOptions.cs
@@ -1,12 +1,12 @@
-using System;
-using System.Collections.Generic;
+using System.Text.Json.Serialization;
+using PuppeteerSharp.Helpers.Json;
 
 namespace PuppeteerSharp.Media
 {
     /// <summary>
     /// margin options used in <see cref="PdfOptions"/>.
     /// </summary>
-    public class MarginOptions : IEquatable<MarginOptions>
+    public record MarginOptions
     {
         /// <summary>
         /// Initializes a new instance of the <see cref="PuppeteerSharp.Media.MarginOptions"/> class.
@@ -18,61 +18,25 @@ public MarginOptions()
         /// <summary>
         /// Top margin, accepts values labeled with units.
         /// </summary>
-        public string Top { get; set; }
+        [JsonConverter(typeof(PrimitiveTypeConverter))]
+        public object Top { get; set; }
 
         /// <summary>
         /// Left margin, accepts values labeled with units.
         /// </summary>
-        public string Left { get; set; }
+        [JsonConverter(typeof(PrimitiveTypeConverter))]
+        public object Left { get; set; }
 
         /// <summary>
         /// Bottom margin, accepts values labeled with units.
         /// </summary>
-        public string Bottom { get; set; }
+        [JsonConverter(typeof(PrimitiveTypeConverter))]
+        public object Bottom { get; set; }
 
         /// <summary>
         /// Right margin, accepts values labeled with units.
         /// </summary>
-        public string Right { get; set; }
-
-        /// <summary>Overriding == operator for <see cref="MarginOptions"/>.</summary>
-        /// <param name="left">the value to compare against <paramref name="right" />.</param>
-        /// <param name="right">the value to compare against <paramref name="left" />.</param>
-        /// <returns><c>true</c> if the two instances are equal to the same value.</returns>
-        public static bool operator ==(MarginOptions left, MarginOptions right)
-            => EqualityComparer<MarginOptions>.Default.Equals(left, right);
-
-        /// <summary>Overriding != operator for <see cref="MarginOptions"/>.</summary>
-        /// <param name="left">the value to compare against <paramref name="right" />.</param>
-        /// <param name="right">the value to compare against <paramref name="left" />.</param>
-        /// <returns><c>true</c> if the two instances are not equal to the same value.</returns>
-        public static bool operator !=(MarginOptions left, MarginOptions right) => !(left == right);
-
-        /// <inheritdoc/>
-        public override bool Equals(object obj)
-        {
-            if (obj == null || GetType() != obj.GetType())
-            {
-                return false;
-            }
-
-            return Equals((MarginOptions)obj);
-        }
-
-        /// <inheritdoc/>
-        public bool Equals(MarginOptions options)
-            => options != null &&
-                   Top == options.Top &&
-                   Left == options.Left &&
-                   Bottom == options.Bottom &&
-                   Right == options.Right;
-
-        /// <inheritdoc/>
-        public override int GetHashCode()
-            => -481391125
-                ^ EqualityComparer<string>.Default.GetHashCode(Top)
-                ^ EqualityComparer<string>.Default.GetHashCode(Left)
-                ^ EqualityComparer<string>.Default.GetHashCode(Bottom)
-                ^ EqualityComparer<string>.Default.GetHashCode(Right);
+        [JsonConverter(typeof(PrimitiveTypeConverter))]
+        public object Right { get; set; }
     }
 }
diff --git a/lib/PuppeteerSharp/Media/PaperFormat.cs b/lib/PuppeteerSharp/Media/PaperFormat.cs
@@ -1,13 +1,10 @@
-using System;
-using System.Collections.Generic;
-
 namespace PuppeteerSharp.Media
 {
     /// <summary>
     /// Paper format.
     /// </summary>
     /// <seealso cref="PdfOptions.Format"/>
-    public class PaperFormat : IEquatable<PaperFormat>
+    public record PaperFormat
     {
         /// <summary>
         /// Initializes a new instance of the <see cref="PaperFormat"/> class.
@@ -87,41 +84,5 @@ public PaperFormat(decimal width, decimal height)
         /// </summary>
         /// <value>The Height.</value>
         public decimal Height { get; set; }
-
-        /// <summary>Overriding == operator for <see cref="PaperFormat"/>. </summary>
-        /// <param name="left">the value to compare against <paramref name="right" />.</param>
-        /// <param name="right">the value to compare against <paramref name="left" />.</param>
-        /// <returns><c>true</c> if the two instances are equal to the same value.</returns>
-        public static bool operator ==(PaperFormat left, PaperFormat right)
-            => EqualityComparer<PaperFormat>.Default.Equals(left, right);
-
-        /// <summary>Overriding != operator for <see cref="PaperFormat"/>. </summary>
-        /// <param name="left">the value to compare against <paramref name="right" />.</param>
-        /// <param name="right">the value to compare against <paramref name="left" />.</param>
-        /// <returns><c>true</c> if the two instances are not equal to the same value.</returns>
-        public static bool operator !=(PaperFormat left, PaperFormat right) => !(left == right);
-
-        /// <inheritdoc/>
-        public override bool Equals(object obj)
-        {
-            if (obj == null || GetType() != obj.GetType())
-            {
-                return false;
-            }
-
-            return Equals((PaperFormat)obj);
-        }
-
-        /// <inheritdoc/>
-        public bool Equals(PaperFormat format)
-            => format != null &&
-                   Width == format.Width &&
-                   Height == format.Height;
-
-        /// <inheritdoc/>
-        public override int GetHashCode()
-            => 859600377
-                ^ Width.GetHashCode()
-                ^ Height.GetHashCode();
     }
 }
diff --git a/lib/PuppeteerSharp/PdfOptions.cs b/lib/PuppeteerSharp/PdfOptions.cs
@@ -1,3 +1,5 @@
+using System.Text.Json.Serialization;
+using PuppeteerSharp.Helpers.Json;
 using PuppeteerSharp.Media;
 
 namespace PuppeteerSharp
@@ -67,11 +69,13 @@ public PdfOptions()
         /// <summary>
         /// Paper width, accepts values labeled with units.
         /// </summary>
+        [JsonConverter(typeof(PrimitiveTypeConverter))]
         public object Width { get; set; }
 
         /// <summary>
         /// Paper height, accepts values labeled with units.
         /// </summary>
+        [JsonConverter(typeof(PrimitiveTypeConverter))]
         public object Height { get; set; }
 
         /// <summary>
