From f976e2c4f4fcf7e53d4c9af1e9e2592244971816 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Fri, 28 Nov 2025 01:20:03 -0500 Subject: [PATCH 1/4] [dotnet] Enable Option to work when no value is provided --- .../BrowsingContext/BrowsingContextModule.cs | 2 +- .../BrowsingContext/SetViewportCommand.cs | 12 ++-- .../BiDi/Json/BiDiJsonSerializerContext.cs | 1 + .../BiDi/Json/Converters/OptionalConverter.cs | 51 -------------- .../Converters/OptionalConverterFactory.cs | 66 +++++++++++++++++++ dotnet/src/webdriver/BiDi/Optional.cs | 15 +++-- .../BrowsingContext/BrowsingContextTest.cs | 2 +- 7 files changed, 87 insertions(+), 62 deletions(-) delete mode 100644 dotnet/src/webdriver/BiDi/Json/Converters/OptionalConverter.cs create mode 100644 dotnet/src/webdriver/BiDi/Json/Converters/OptionalConverterFactory.cs diff --git a/dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContextModule.cs b/dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContextModule.cs index 636634bb0b1d2..27c5fa80569f0 100644 --- a/dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContextModule.cs +++ b/dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContextModule.cs @@ -82,7 +82,7 @@ public async Task ReloadAsync(BrowsingContext context, ReloadOptio public async Task SetViewportAsync(BrowsingContext context, SetViewportOptions? options = null) { - var @params = new SetViewportParameters(context, options?.Viewport, options?.DevicePixelRatio); + var @params = new SetViewportParameters(context, options?.Viewport ?? Optional.None, options?.DevicePixelRatio ?? Optional.None); return await Broker.ExecuteCommandAsync(new SetViewportCommand(@params), options, JsonContext.SetViewportCommand, JsonContext.SetViewportResult).ConfigureAwait(false); } diff --git a/dotnet/src/webdriver/BiDi/BrowsingContext/SetViewportCommand.cs b/dotnet/src/webdriver/BiDi/BrowsingContext/SetViewportCommand.cs index d11f1ebe8e84b..20b19b98e473e 100644 --- a/dotnet/src/webdriver/BiDi/BrowsingContext/SetViewportCommand.cs +++ b/dotnet/src/webdriver/BiDi/BrowsingContext/SetViewportCommand.cs @@ -17,7 +17,6 @@ // under the License. // -using OpenQA.Selenium.BiDi.Json.Converters; using System.Text.Json.Serialization; namespace OpenQA.Selenium.BiDi.BrowsingContext; @@ -25,13 +24,18 @@ namespace OpenQA.Selenium.BiDi.BrowsingContext; internal sealed class SetViewportCommand(SetViewportParameters @params) : Command(@params, "browsingContext.setViewport"); -internal sealed record SetViewportParameters(BrowsingContext Context, [property: JsonConverter(typeof(OptionalConverter))] Optional? Viewport, [property: JsonConverter(typeof(OptionalConverter))] Optional? DevicePixelRatio) : Parameters; +internal sealed record SetViewportParameters( + BrowsingContext Context, + [property: JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] Optional Viewport, + [property: JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] Optional DevicePixelRatio) : Parameters; public sealed class SetViewportOptions : CommandOptions { - public Optional? Viewport { get; set; } + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] + public Optional Viewport { get; set; } - public Optional? DevicePixelRatio { get; set; } + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] + public Optional DevicePixelRatio { get; set; } } public readonly record struct Viewport(long Width, long Height); diff --git a/dotnet/src/webdriver/BiDi/Json/BiDiJsonSerializerContext.cs b/dotnet/src/webdriver/BiDi/Json/BiDiJsonSerializerContext.cs index 046e3a4938b96..4636ed81a4d56 100644 --- a/dotnet/src/webdriver/BiDi/Json/BiDiJsonSerializerContext.cs +++ b/dotnet/src/webdriver/BiDi/Json/BiDiJsonSerializerContext.cs @@ -121,6 +121,7 @@ namespace OpenQA.Selenium.BiDi.Json; [JsonSerializable(typeof(BrowsingContext.SetViewportResult))] [JsonSerializable(typeof(BrowsingContext.TraverseHistoryCommand))] [JsonSerializable(typeof(BrowsingContext.TraverseHistoryResult))] +[JsonSerializable(typeof(BrowsingContext.Viewport?))] [JsonSerializable(typeof(BrowsingContext.BrowsingContextInfo))] [JsonSerializable(typeof(BrowsingContext.DownloadWillBeginEventArgs))] diff --git a/dotnet/src/webdriver/BiDi/Json/Converters/OptionalConverter.cs b/dotnet/src/webdriver/BiDi/Json/Converters/OptionalConverter.cs deleted file mode 100644 index 0fa368adce1f9..0000000000000 --- a/dotnet/src/webdriver/BiDi/Json/Converters/OptionalConverter.cs +++ /dev/null @@ -1,51 +0,0 @@ -// -// Licensed to the Software Freedom Conservancy (SFC) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The SFC licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. -// - -using System; -using System.Text.Json; -using System.Text.Json.Serialization; - -namespace OpenQA.Selenium.BiDi.Json.Converters; - -public sealed class OptionalConverter : JsonConverter> -{ - public override Optional Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) - { - if (reader.TokenType == JsonTokenType.Null) - { - reader.Read(); // consume null - return new Optional(default!); - } - - T value = JsonSerializer.Deserialize(ref reader, options)!; - return new Optional(value); - } - - public override void Write(Utf8JsonWriter writer, Optional value, JsonSerializerOptions options) - { - if (value.TryGetValue(out var optionalValue)) - { - JsonSerializer.Serialize(writer, optionalValue, options); - } - else - { - writer.WriteNullValue(); - } - } -} diff --git a/dotnet/src/webdriver/BiDi/Json/Converters/OptionalConverterFactory.cs b/dotnet/src/webdriver/BiDi/Json/Converters/OptionalConverterFactory.cs new file mode 100644 index 0000000000000..1ee3c7898c1a0 --- /dev/null +++ b/dotnet/src/webdriver/BiDi/Json/Converters/OptionalConverterFactory.cs @@ -0,0 +1,66 @@ +// +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +using System; +using System.Text.Json; +using System.Text.Json.Serialization; + +namespace OpenQA.Selenium.BiDi.Json.Converters; + +internal sealed class OptionalConverterFactory : JsonConverterFactory +{ + public override bool CanConvert(Type typeToConvert) => typeToConvert.IsGenericType && typeToConvert.GetGenericTypeDefinition() == typeof(Optional<>); + + public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options) + { + var converter = (JsonConverter?)Activator.CreateInstance(typeof(OptionalConverter<>).MakeGenericType(typeToConvert.GenericTypeArguments[0])); + + return converter; + } + + internal sealed class OptionalConverter : JsonConverter> + { + public override bool HandleNull => true; + + public override Optional Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType == JsonTokenType.Null) + { + reader.Read(); // consume null + return new Optional(default!); + } + + T value = JsonSerializer.Deserialize(ref reader, options)!; + return new Optional(value); + } + + public override void Write(Utf8JsonWriter writer, Optional value, JsonSerializerOptions options) + { + if (value.TryGetValue(out var optionalValue)) + { + JsonSerializer.Serialize(writer, optionalValue, options); + } + else + { + throw new JsonException("This property should be annotated with [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)]"); + } + } + } +} + diff --git a/dotnet/src/webdriver/BiDi/Optional.cs b/dotnet/src/webdriver/BiDi/Optional.cs index e31fbbce2c9c6..94aac16d37a10 100644 --- a/dotnet/src/webdriver/BiDi/Optional.cs +++ b/dotnet/src/webdriver/BiDi/Optional.cs @@ -17,20 +17,23 @@ // under the License. // +using OpenQA.Selenium.BiDi.Json.Converters; using System; +using System.Text.Json.Serialization; namespace OpenQA.Selenium.BiDi; +[JsonConverter(typeof(OptionalConverterFactory))] public readonly record struct Optional { - private readonly T _value; + private readonly T? _value; public bool HasValue { get; } - public T Value => HasValue + public T? Value => HasValue ? _value - : throw new InvalidOperationException("Optional has no value. Check IsSet first."); + : throw new InvalidOperationException($"Optional has no value. Check {nameof(HasValue)} first."); - public Optional(T value) + public Optional(T? value) { _value = value; HasValue = true; @@ -42,6 +45,8 @@ public bool TryGetValue(out T value) return HasValue; } + public static Optional None => default; + // implicit conversion from T -> Optional - public static implicit operator Optional(T value) => new(value); + public static implicit operator Optional(T? value) => new(value); } diff --git a/dotnet/test/common/BiDi/BrowsingContext/BrowsingContextTest.cs b/dotnet/test/common/BiDi/BrowsingContext/BrowsingContextTest.cs index 9a62004bede8b..ebfc20625f563 100644 --- a/dotnet/test/common/BiDi/BrowsingContext/BrowsingContextTest.cs +++ b/dotnet/test/common/BiDi/BrowsingContext/BrowsingContextTest.cs @@ -316,7 +316,7 @@ public async Task CanSetViewport() Assert.That(await GetHeightAsync(), Is.EqualTo(300)); await context.SetViewportAsync(new() { Viewport = new Viewport(250, 300) }); - await context.SetViewportAsync(new() { Viewport = default }); // Sends nothing + await context.SetViewportAsync(new() { Viewport = Optional.None }); // Sends nothing Assert.That(await GetWidthAsync(), Is.EqualTo(250)); Assert.That(await GetHeightAsync(), Is.EqualTo(300)); From 82c9f91346f14a9d1a8d88d07e04820d5cbcc49c Mon Sep 17 00:00:00 2001 From: Michael Render Date: Sun, 30 Nov 2025 15:09:52 -0500 Subject: [PATCH 2/4] Remove superfluous serialization type --- dotnet/src/webdriver/BiDi/Json/BiDiJsonSerializerContext.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/dotnet/src/webdriver/BiDi/Json/BiDiJsonSerializerContext.cs b/dotnet/src/webdriver/BiDi/Json/BiDiJsonSerializerContext.cs index 4636ed81a4d56..046e3a4938b96 100644 --- a/dotnet/src/webdriver/BiDi/Json/BiDiJsonSerializerContext.cs +++ b/dotnet/src/webdriver/BiDi/Json/BiDiJsonSerializerContext.cs @@ -121,7 +121,6 @@ namespace OpenQA.Selenium.BiDi.Json; [JsonSerializable(typeof(BrowsingContext.SetViewportResult))] [JsonSerializable(typeof(BrowsingContext.TraverseHistoryCommand))] [JsonSerializable(typeof(BrowsingContext.TraverseHistoryResult))] -[JsonSerializable(typeof(BrowsingContext.Viewport?))] [JsonSerializable(typeof(BrowsingContext.BrowsingContextInfo))] [JsonSerializable(typeof(BrowsingContext.DownloadWillBeginEventArgs))] From 875b0c1e4f5a4863fa2a62520503bd6c1c020237 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Sun, 30 Nov 2025 15:11:26 -0500 Subject: [PATCH 3/4] [dotnet] Remove serialization parameters from non-serializable options type --- dotnet/src/webdriver/BiDi/BrowsingContext/SetViewportCommand.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/dotnet/src/webdriver/BiDi/BrowsingContext/SetViewportCommand.cs b/dotnet/src/webdriver/BiDi/BrowsingContext/SetViewportCommand.cs index 20b19b98e473e..fe8cd67254b1c 100644 --- a/dotnet/src/webdriver/BiDi/BrowsingContext/SetViewportCommand.cs +++ b/dotnet/src/webdriver/BiDi/BrowsingContext/SetViewportCommand.cs @@ -31,10 +31,8 @@ internal sealed record SetViewportParameters( public sealed class SetViewportOptions : CommandOptions { - [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] public Optional Viewport { get; set; } - [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] public Optional DevicePixelRatio { get; set; } } From c28d925cfba57640bcbf256648c2190992d88c6e Mon Sep 17 00:00:00 2001 From: Michael Render Date: Sun, 30 Nov 2025 23:37:51 -0500 Subject: [PATCH 4/4] [dotnet] Fill in gaps in the type closure for Optional-containing types --- .../BiDi/BrowsingContext/BrowsingContextModule.cs | 2 ++ .../Json/Converters/OptionalConverterFactory.cs | 13 ++++++------- dotnet/src/webdriver/BiDi/Optional.cs | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContextModule.cs b/dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContextModule.cs index 12040bda7479e..c8a7b11a23159 100644 --- a/dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContextModule.cs +++ b/dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContextModule.cs @@ -292,4 +292,6 @@ protected override void Initialize(JsonSerializerOptions options) [JsonSerializable(typeof(NavigationInfo))] [JsonSerializable(typeof(UserPromptOpenedEventArgs))] [JsonSerializable(typeof(UserPromptClosedEventArgs))] +[JsonSerializable(typeof(Viewport?))] +[JsonSerializable(typeof(double?))] internal partial class BrowsingContextJsonSerializerContext : JsonSerializerContext; diff --git a/dotnet/src/webdriver/BiDi/Json/Converters/OptionalConverterFactory.cs b/dotnet/src/webdriver/BiDi/Json/Converters/OptionalConverterFactory.cs index 1ee3c7898c1a0..beebecb4c780b 100644 --- a/dotnet/src/webdriver/BiDi/Json/Converters/OptionalConverterFactory.cs +++ b/dotnet/src/webdriver/BiDi/Json/Converters/OptionalConverterFactory.cs @@ -20,6 +20,7 @@ using System; using System.Text.Json; using System.Text.Json.Serialization; +using System.Text.Json.Serialization.Metadata; namespace OpenQA.Selenium.BiDi.Json.Converters; @@ -40,13 +41,9 @@ internal sealed class OptionalConverter : JsonConverter> public override Optional Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - if (reader.TokenType == JsonTokenType.Null) - { - reader.Read(); // consume null - return new Optional(default!); - } + var converter = (JsonTypeInfo)options.GetTypeInfo(typeof(T)); - T value = JsonSerializer.Deserialize(ref reader, options)!; + T? value = JsonSerializer.Deserialize(ref reader, converter); return new Optional(value); } @@ -54,7 +51,9 @@ public override void Write(Utf8JsonWriter writer, Optional value, JsonSeriali { if (value.TryGetValue(out var optionalValue)) { - JsonSerializer.Serialize(writer, optionalValue, options); + var converter = (JsonTypeInfo)options.GetTypeInfo(typeof(T?)); + + JsonSerializer.Serialize(writer, optionalValue, converter); } else { diff --git a/dotnet/src/webdriver/BiDi/Optional.cs b/dotnet/src/webdriver/BiDi/Optional.cs index 94aac16d37a10..51da02ff01537 100644 --- a/dotnet/src/webdriver/BiDi/Optional.cs +++ b/dotnet/src/webdriver/BiDi/Optional.cs @@ -39,7 +39,7 @@ public Optional(T? value) HasValue = true; } - public bool TryGetValue(out T value) + public bool TryGetValue(out T? value) { value = _value; return HasValue;