Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dotnet] Annotate nullability on more of WebElement #15230

Open
wants to merge 11 commits into
base: trunk
Choose a base branch
from
+56 −23
Open

[dotnet] Annotate nullability on more of WebElement #15230

Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
2ea99a3
[dotnet] Annotate nullability on more of `WebElement`
RenderMichael Feb 4, 2025
696832f
Handle nullability of GetCssValue
RenderMichael Feb 4, 2025
e8c53af
Implement nullability on more of `WebElement`
RenderMichael Feb 4, 2025
bca2dc8
fix typo
RenderMichael Feb 4, 2025
c55eb00
improve error message
RenderMichael Feb 4, 2025
d895b6b
Merge branch 'trunk' into webelement-nullable-2
RenderMichael Feb 4, 2025
7b0f681
Merge branch 'trunk' into webelement-nullable-2
RenderMichael Feb 6, 2025
c07436f
Use new `EnsureValueIsNotNull` hlper
RenderMichael Feb 6, 2025
bb69c82
Merge branch 'trunk' into webelement-nullable-2
RenderMichael Feb 12, 2025
fd977e0
Merge branch 'trunk' into webelement-nullable-2
RenderMichael Feb 12, 2025
cabb1e8
Merge branch 'trunk' into webelement-nullable-2
RenderMichael Feb 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 49 additions & 15 deletions dotnet/src/webdriver/WebElement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ public class WebElement : IWebElement, IFindsElement, IWrapsDriver, ILocatable,
/// </summary>
public const string ElementReferencePropertyName = "element-6066-11e4-a52e-4f735466cecf";

#nullable enable

private readonly WebDriver driver;

/// <summary>
Expand All @@ -59,6 +61,8 @@ public WebElement(WebDriver parentDriver, string id)
/// </summary>
public IWebDriver WrappedDriver => this.driver;

#nullable restore

/// <summary>
/// Gets the tag name of this element.
/// </summary>
Expand Down Expand Up @@ -99,6 +103,8 @@ public virtual string Text
}
}

#nullable enable

/// <summary>
/// Gets a value indicating whether or not this element is enabled.
/// </summary>
Expand Down Expand Up @@ -151,7 +157,11 @@ public virtual Point Location

Response commandResponse = this.Execute(DriverCommand.GetElementRect, parameters);

Dictionary<string, object> rawPoint = (Dictionary<string, object>)commandResponse.Value;
if (commandResponse.Value is not Dictionary<string, object?> rawPoint)
{
throw new WebDriverException($"GetElementRect command was successful, but response was not an object: {commandResponse.Value}");
}

int x = Convert.ToInt32(rawPoint["x"], CultureInfo.InvariantCulture);
int y = Convert.ToInt32(rawPoint["y"], CultureInfo.InvariantCulture);
return new Point(x, y);
Expand All @@ -171,7 +181,11 @@ public virtual Size Size

Response commandResponse = this.Execute(DriverCommand.GetElementRect, parameters);

Dictionary<string, object> rawSize = (Dictionary<string, object>)commandResponse.Value;
if (commandResponse.Value is not Dictionary<string, object?> rawSize)
{
throw new WebDriverException($"GetElementRect command was successful, but response was not an object: {commandResponse.Value}");
}

int width = Convert.ToInt32(rawSize["width"], CultureInfo.InvariantCulture);
int height = Convert.ToInt32(rawSize["height"], CultureInfo.InvariantCulture);
return new Size(width, height);
Expand Down Expand Up @@ -207,7 +221,7 @@ public virtual Point LocationOnScreenOnceScrolledIntoView
{
get
{
object scriptResponse = this.driver.ExecuteScript("var rect = arguments[0].getBoundingClientRect(); return {'x': rect.left, 'y': rect.top};", this);
object scriptResponse = this.driver.ExecuteScript("var rect = arguments[0].getBoundingClientRect(); return {'x': rect.left, 'y': rect.top};", this)!;

Dictionary<string, object> rawLocation = (Dictionary<string, object>)scriptResponse;

Expand All @@ -217,6 +231,8 @@ public virtual Point LocationOnScreenOnceScrolledIntoView
}
}

#nullable restore

/// <summary>
/// Gets the computed accessible label of this element.
/// </summary>
Expand Down Expand Up @@ -253,6 +269,8 @@ public virtual string ComputedAccessibleRole
}
}

#nullable enable

/// <summary>
/// Gets the coordinates identifying the location of this element using
/// various frames of reference.
Expand Down Expand Up @@ -318,6 +336,7 @@ public virtual void Click()
/// </summary>
/// <param name="by">The locating mechanism to use.</param>
/// <returns>The first matching <see cref="IWebElement"/> on the current context.</returns>
/// <exception cref="ArgumentNullException">If <paramref name="by"/> is <see langword="null"/>.</exception>
/// <exception cref="NoSuchElementException">If no element matches the criteria.</exception>
public virtual IWebElement FindElement(By by)
{
Expand All @@ -329,6 +348,8 @@ public virtual IWebElement FindElement(By by)
return by.FindElement(this);
}

#nullable restore

/// <summary>
/// Finds a child element matching the given mechanism and value.
/// </summary>
Expand Down Expand Up @@ -382,6 +403,8 @@ public virtual ReadOnlyCollection<IWebElement> FindElements(string mechanism, st
return this.driver.GetElementsFromResponse(commandResponse);
}

#nullable enable

/// <summary>
/// Gets the value of the specified attribute or property for this element.
/// </summary>
Expand Down Expand Up @@ -419,15 +442,14 @@ public virtual ReadOnlyCollection<IWebElement> FindElements(string mechanism, st
/// via JavaScript.
/// </remarks>
/// <exception cref="StaleElementReferenceException">Thrown when the target element is no longer valid in the document DOM.</exception>
public virtual string GetAttribute(string attributeName)
public virtual string? GetAttribute(string attributeName)
{
Response commandResponse = null;
string attributeValue = string.Empty;
Dictionary<string, object> parameters = new Dictionary<string, object>();
string atom = GetAtom("get-attribute.js");
parameters.Add("script", atom);
parameters.Add("args", new object[] { ((IWebDriverObjectReference)this).ToDictionary(), attributeName });
commandResponse = this.Execute(DriverCommand.ExecuteScript, parameters);

Response commandResponse = Execute(DriverCommand.ExecuteScript, parameters);


// Normalize string values of boolean results as lowercase.
Expand All @@ -452,7 +474,7 @@ public virtual string GetAttribute(string attributeName)
/// of an IDL property of the element, either use the <see cref="GetAttribute(string)"/>
/// method or the <see cref="GetDomProperty(string)"/> method.
/// </remarks>
public virtual string GetDomAttribute(string attributeName)
public virtual string? GetDomAttribute(string attributeName)
{
Dictionary<string, object> parameters = new Dictionary<string, object>();
parameters.Add("id", this.Id);
Expand All @@ -470,7 +492,7 @@ public virtual string GetDomAttribute(string attributeName)
/// <returns>The JavaScript property's current value. Returns a <see langword="null"/> if the
/// value is not set or the property does not exist.</returns>
/// <exception cref="StaleElementReferenceException">Thrown when the target element is no longer valid in the document DOM.</exception>
public virtual string GetDomProperty(string propertyName)
public virtual string? GetDomProperty(string propertyName)
{
Dictionary<string, object> parameters = new Dictionary<string, object>();
parameters.Add("id", this.Id);
Expand All @@ -493,12 +515,12 @@ public virtual ISearchContext GetShadowRoot()
parameters.Add("id", this.Id);

Response commandResponse = this.Execute(DriverCommand.GetElementShadowRoot, parameters);
if (commandResponse.Value is not Dictionary<string, object> shadowRootDictionary)
if (commandResponse.Value is not Dictionary<string, object?> shadowRootDictionary)
{
throw new WebDriverException("Get shadow root command succeeded, but response value does not represent a shadow root.");
}

if (!ShadowRoot.TryCreate(this.driver, shadowRootDictionary, out ShadowRoot shadowRoot))
if (!ShadowRoot.TryCreate(this.driver, shadowRootDictionary, out ShadowRoot? shadowRoot))
{
throw new WebDriverException("Get shadow root command succeeded, but response value does not have a shadow root key value.");
}
Expand All @@ -524,7 +546,13 @@ public virtual string GetCssValue(string propertyName)
parameters.Add("name", propertyName);

Response commandResponse = this.Execute(DriverCommand.GetElementValueOfCssProperty, parameters);
return commandResponse.Value.ToString();

if (commandResponse.Value is null)
{
throw new WebDriverException("GetElementValueOfCssProperty command returned a successful result, but contained no data");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should test it carefully.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to be very careful with this, to change no behavior. I read the spec, it says this will always be a string.

The only reason I am comfortable with this change, is because it would previously be a NullReferenceException if this were null. This way, users have something better to report if there is a bug here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very good insight. Please give me a chance to revisit it.

}

return commandResponse.Value.ToString()!;
}

/// <summary>
Expand All @@ -538,7 +566,7 @@ public virtual Screenshot GetScreenshot()

// Get the screenshot as base64.
Response screenshotResponse = this.Execute(DriverCommand.ElementScreenshot, parameters);
string base64 = screenshotResponse.Value.ToString();
string base64 = screenshotResponse.Value?.ToString() ?? throw new WebDriverException("ElementScreenshot command returned successfully, but with no response");

// ... and convert it.
return new Screenshot(base64);
Expand Down Expand Up @@ -597,7 +625,7 @@ public virtual void SendKeys(string text)
/// <exception cref="StaleElementReferenceException">Thrown when the target element is no longer valid in the document DOM.</exception>
public virtual void Submit()
{
string elementType = this.GetAttribute("type");
string? elementType = this.GetAttribute("type");
if (elementType != null && elementType == "submit")
{
this.Click();
Expand Down Expand Up @@ -641,7 +669,7 @@ public override int GetHashCode()
/// </summary>
/// <param name="obj">Object to compare against</param>
/// <returns>A boolean if it is equal or not</returns>
public override bool Equals(object obj)
public override bool Equals(object? obj)
{
if (obj is not IWebElement other)
{
Expand Down Expand Up @@ -676,6 +704,8 @@ Dictionary<string, object> IWebDriverObjectReference.ToDictionary()
return elementDictionary;
}

#nullable restore

/// <summary>
/// Executes a command on this element using the specified parameters.
/// </summary>
Expand All @@ -687,6 +717,8 @@ protected virtual Response Execute(string commandToExecute, Dictionary<string, o
return this.driver.InternalExecute(commandToExecute, parameters);
}

#nullable enable

private static string GetAtom(string atomResourceName)
{
string atom = string.Empty;
Expand All @@ -703,6 +735,8 @@ private static string GetAtom(string atomResourceName)
return wrappedAtom;
}

#nullable restore

private string UploadFile(string localFile)
{
string base64zip;
Expand Down
Loading