Fix CSS selector support for numeric IDs and escape sequences by sinpru · Pull Request #150 · FlaUI/FlaUI.WebDriver

sinpru · 2025-11-13T04:17:15Z

aristotelos · 2025-12-06T11:06:58Z

+
+ private (string strategy, string value) ParseCssSelector(string cssSelector)
+ {
+ if (cssSelector.StartsWith("#"))


This is not sufficient to judge that it is a simple CSS ID selector. To be sure, shouldn't a regex be used that checks that the complete cssSelector string matches a simple ID selector? It may be that the user sends a compound selector instead, and we should have a proper error message in that case that this is not supported.
The same comment holds for all parses below. To check that a regex matches the string fully, start with ^ and end with $.

aristotelos · 2025-12-08T05:36:13Z

Would it be possible to add a unit test to e.g. ConditionParser to check the case that is currently not working?

sinpru · 2025-12-14T15:39:20Z

Actually I found out that my approach to fixing the CSS selector inside the controller was a mistake. I've refactored this to handle CSS selector parsing in ConditionParser instead.

aristotelos · 2025-12-08T05:25:15Z

- }
-
- var nameMatch = Regex.Match(cssSelector, @"\*?\[name\s*=\s*""(.+?)""\]", RegexOptions.IgnoreCase);
+ var nameMatch = Regex.Match(cssSelector, @"^\*?\[name\s*=\s*""(.+?)""\]$", RegexOptions.IgnoreCase);


Couldn't this still match [name="test"][class="test2"] because of the .+ expression? If so, I suggest [^""]+ instead. Please add a unit test to validate that [name="test"][class="test2"] throws as not supported.

You're right, the .+ is too greedy, I've added a unit test that validates compound selectors throws as intened.

aristotelos · 2025-12-08T05:35:05Z

 { 
 element = await Wait.Until(() => startNode().FindFirstByXPath(findElementRequest.Value), element => element != null, session.ImplicitWaitTimeout);
 }
+ else if (findElementRequest.Using == "css selector")


I've had a closer look and it seems that the ConditionParser should already allow escaped characters too. Instead of adding another case here, can't we fix the ConditionParser regular expressions if they fail?

aristotelos · 2025-12-17T05:48:08Z

@@ -31,19 +29,24 @@ public class ConditionParser : IConditionParser
 /// - Attribute equals attribute (e.g. `[name=value]`) not supported.


Also update the comment above Unicode escape characters or escape characters in the attribute name are not supported. -> Escape characters in the attribute name are not supported.

aristotelos · 2025-12-17T05:49:14Z

 /// - Multiple selectors are not supported.
 /// </summary>
- private static Regex SimpleCssIdSelectorRegex = new Regex(@"^#(?<name>(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f]))+)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssIdSelectorRegex = new Regex(@"^#(?<name>(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))+)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

The \s? is needed to make the trailing space optional per CSS spec. Unicode escapes like \34 can be followed by an optional space (\34 b or \34b are both valid depending on context).

aristotelos · 2025-12-17T05:49:21Z

 /// - Multiple selectors are not supported.
 /// </summary>
- private static Regex SimpleCssClassSelectorRegex = new Regex(@"^\.(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f]))*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssClassSelectorRegex = new Regex(@"^\.(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

This goes the same as the one above

aristotelos · 2025-12-17T05:49:27Z

 /// - ~= or |= not supported.
 /// </summary>
- private static Regex SimpleCssAttributeSelectorRegex = new Regex(@"^\*?\[\s*(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377])*)\s*=\s*(?<string>(?<string1>""(?<string1value>([^\n\r\f\\""]|(?<escape>\\[^\r\n\f0-9a-f]))*)"")|(?<string2>'(?<string2value>([^\n\r\f\\']|(?<escape>\\[^\r\n\f0-9a-f]))*)'))\s*\]$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssAttributeSelectorRegex = new Regex(@"^\*?\[\s*(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377])*)\s*=\s*(?<string>(?<string1>""(?<string1value>([^\n\r\f\\""]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)"")|(?<string2>'(?<string2value>([^\n\r\f\\']|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)'))\s*\]$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

aristotelos · 2025-12-17T05:50:39Z

+ [Test]
+ public void ParseCondition_CssSelectorWithSimpleNumericId_ReturnsAutomationIdCondition()
+ {
+ var cssSelector = @"#\31 ";


Isn't "#\31" without the trailing space a better example for this case?

The trailing space is needed when the next character is a hex digit. Without it #\31b would be parsed as a 3-character escape (\31b) instead of \31 followed by b.

aristotelos · 2025-12-17T05:52:21Z

 {
 public class ConditionParserTests
 {
- [TestCase("[name=\"2\"]")]


Why were the ParseCondition_ByCssAttributeName_ReturnsCondition test cases removed?

The old test was removed because it was testing the same functionality now covered by ParseCondition_CssSelectorWithEscapedSpecialChars_ReturnsNameCondition. Should I keep both for coverage?

aristotelos · 2025-12-17T05:56:17Z

Please squash the commits in this merge request together.

E.g. with git rebase -i or with:

git reset main
git commit

sinpru · 2025-11-13T04:17:15Z

Fixes#138

aristotelos · 2025-12-06T11:06:58Z

+
+ private (string strategy, string value) ParseCssSelector(string cssSelector)
+ {
+ if (cssSelector.StartsWith("#"))


This is not sufficient to judge that it is a simple CSS ID selector. To be sure, shouldn't a regex be used that checks that the complete cssSelector string matches a simple ID selector? It may be that the user sends a compound selector instead, and we should have a proper error message in that case that this is not supported.
The same comment holds for all parses below. To check that a regex matches the string fully, start with ^ and end with $.

aristotelos · 2025-12-08T05:36:13Z

Would it be possible to add a unit test to e.g. ConditionParser to check the case that is currently not working?

sinpru · 2025-12-14T15:39:20Z

Actually I found out that my approach to fixing the CSS selector inside the controller was a mistake. I've refactored this to handle CSS selector parsing in ConditionParser instead.

aristotelos · 2025-12-08T05:25:15Z

- }
-
- var nameMatch = Regex.Match(cssSelector, @"\*?\[name\s*=\s*""(.+?)""\]", RegexOptions.IgnoreCase);
+ var nameMatch = Regex.Match(cssSelector, @"^\*?\[name\s*=\s*""(.+?)""\]$", RegexOptions.IgnoreCase);


Couldn't this still match [name="test"][class="test2"] because of the .+ expression? If so, I suggest [^""]+ instead. Please add a unit test to validate that [name="test"][class="test2"] throws as not supported.

You're right, the .+ is too greedy, I've added a unit test that validates compound selectors throws as intened.

aristotelos · 2025-12-08T05:35:05Z

 { 
 element = await Wait.Until(() => startNode().FindFirstByXPath(findElementRequest.Value), element => element != null, session.ImplicitWaitTimeout);
 }
+ else if (findElementRequest.Using == "css selector")


I've had a closer look and it seems that the ConditionParser should already allow escaped characters too. Instead of adding another case here, can't we fix the ConditionParser regular expressions if they fail?

aristotelos · 2025-12-17T05:48:08Z

@@ -31,19 +29,24 @@ public class ConditionParser : IConditionParser
 /// - Attribute equals attribute (e.g. `[name=value]`) not supported.


Also update the comment above Unicode escape characters or escape characters in the attribute name are not supported. -> Escape characters in the attribute name are not supported.

aristotelos · 2025-12-17T05:49:14Z

 /// - Multiple selectors are not supported.
 /// </summary>
- private static Regex SimpleCssIdSelectorRegex = new Regex(@"^#(?<name>(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f]))+)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssIdSelectorRegex = new Regex(@"^#(?<name>(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))+)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

The \s? is needed to make the trailing space optional per CSS spec. Unicode escapes like \34 can be followed by an optional space (\34 b or \34b are both valid depending on context).

aristotelos · 2025-12-17T05:49:21Z

 /// - Multiple selectors are not supported.
 /// </summary>
- private static Regex SimpleCssClassSelectorRegex = new Regex(@"^\.(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f]))*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssClassSelectorRegex = new Regex(@"^\.(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

This goes the same as the one above

aristotelos · 2025-12-17T05:49:27Z

 /// - ~= or |= not supported.
 /// </summary>
- private static Regex SimpleCssAttributeSelectorRegex = new Regex(@"^\*?\[\s*(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377])*)\s*=\s*(?<string>(?<string1>""(?<string1value>([^\n\r\f\\""]|(?<escape>\\[^\r\n\f0-9a-f]))*)"")|(?<string2>'(?<string2value>([^\n\r\f\\']|(?<escape>\\[^\r\n\f0-9a-f]))*)'))\s*\]$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssAttributeSelectorRegex = new Regex(@"^\*?\[\s*(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377])*)\s*=\s*(?<string>(?<string1>""(?<string1value>([^\n\r\f\\""]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)"")|(?<string2>'(?<string2value>([^\n\r\f\\']|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)'))\s*\]$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

aristotelos · 2025-12-17T05:50:39Z

+ [Test]
+ public void ParseCondition_CssSelectorWithSimpleNumericId_ReturnsAutomationIdCondition()
+ {
+ var cssSelector = @"#\31 ";


Isn't "#\31" without the trailing space a better example for this case?

The trailing space is needed when the next character is a hex digit. Without it #\31b would be parsed as a 3-character escape (\31b) instead of \31 followed by b.

aristotelos · 2025-12-17T05:52:21Z

 {
 public class ConditionParserTests
 {
- [TestCase("[name=\"2\"]")]


Why were the ParseCondition_ByCssAttributeName_ReturnsCondition test cases removed?

The old test was removed because it was testing the same functionality now covered by ParseCondition_CssSelectorWithEscapedSpecialChars_ReturnsNameCondition. Should I keep both for coverage?

aristotelos · 2025-12-17T05:56:17Z

Please squash the commits in this merge request together.

E.g. with git rebase -i or with:

git reset main
git commit

sinpru · 2025-11-13T04:17:15Z

Fixes#138

aristotelos · 2025-12-06T11:06:58Z

+
+ private (string strategy, string value) ParseCssSelector(string cssSelector)
+ {
+ if (cssSelector.StartsWith("#"))


This is not sufficient to judge that it is a simple CSS ID selector. To be sure, shouldn't a regex be used that checks that the complete cssSelector string matches a simple ID selector? It may be that the user sends a compound selector instead, and we should have a proper error message in that case that this is not supported.
The same comment holds for all parses below. To check that a regex matches the string fully, start with ^ and end with $.

aristotelos · 2025-12-08T05:36:13Z

Would it be possible to add a unit test to e.g. ConditionParser to check the case that is currently not working?

sinpru · 2025-12-14T15:39:20Z

Actually I found out that my approach to fixing the CSS selector inside the controller was a mistake. I've refactored this to handle CSS selector parsing in ConditionParser instead.

aristotelos · 2025-12-08T05:25:15Z

- }
-
- var nameMatch = Regex.Match(cssSelector, @"\*?\[name\s*=\s*""(.+?)""\]", RegexOptions.IgnoreCase);
+ var nameMatch = Regex.Match(cssSelector, @"^\*?\[name\s*=\s*""(.+?)""\]$", RegexOptions.IgnoreCase);


Couldn't this still match [name="test"][class="test2"] because of the .+ expression? If so, I suggest [^""]+ instead. Please add a unit test to validate that [name="test"][class="test2"] throws as not supported.

You're right, the .+ is too greedy, I've added a unit test that validates compound selectors throws as intened.

aristotelos · 2025-12-08T05:35:05Z

 { 
 element = await Wait.Until(() => startNode().FindFirstByXPath(findElementRequest.Value), element => element != null, session.ImplicitWaitTimeout);
 }
+ else if (findElementRequest.Using == "css selector")


I've had a closer look and it seems that the ConditionParser should already allow escaped characters too. Instead of adding another case here, can't we fix the ConditionParser regular expressions if they fail?

aristotelos · 2025-12-17T05:48:08Z

@@ -31,19 +29,24 @@ public class ConditionParser : IConditionParser
 /// - Attribute equals attribute (e.g. `[name=value]`) not supported.


Also update the comment above Unicode escape characters or escape characters in the attribute name are not supported. -> Escape characters in the attribute name are not supported.

aristotelos · 2025-12-17T05:49:14Z

 /// - Multiple selectors are not supported.
 /// </summary>
- private static Regex SimpleCssIdSelectorRegex = new Regex(@"^#(?<name>(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f]))+)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssIdSelectorRegex = new Regex(@"^#(?<name>(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))+)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

The \s? is needed to make the trailing space optional per CSS spec. Unicode escapes like \34 can be followed by an optional space (\34 b or \34b are both valid depending on context).

aristotelos · 2025-12-17T05:49:21Z

 /// - Multiple selectors are not supported.
 /// </summary>
- private static Regex SimpleCssClassSelectorRegex = new Regex(@"^\.(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f]))*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssClassSelectorRegex = new Regex(@"^\.(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

This goes the same as the one above

aristotelos · 2025-12-17T05:49:27Z

 /// - ~= or |= not supported.
 /// </summary>
- private static Regex SimpleCssAttributeSelectorRegex = new Regex(@"^\*?\[\s*(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377])*)\s*=\s*(?<string>(?<string1>""(?<string1value>([^\n\r\f\\""]|(?<escape>\\[^\r\n\f0-9a-f]))*)"")|(?<string2>'(?<string2value>([^\n\r\f\\']|(?<escape>\\[^\r\n\f0-9a-f]))*)'))\s*\]$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssAttributeSelectorRegex = new Regex(@"^\*?\[\s*(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377])*)\s*=\s*(?<string>(?<string1>""(?<string1value>([^\n\r\f\\""]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)"")|(?<string2>'(?<string2value>([^\n\r\f\\']|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)'))\s*\]$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

aristotelos · 2025-12-17T05:50:39Z

+ [Test]
+ public void ParseCondition_CssSelectorWithSimpleNumericId_ReturnsAutomationIdCondition()
+ {
+ var cssSelector = @"#\31 ";


Isn't "#\31" without the trailing space a better example for this case?

The trailing space is needed when the next character is a hex digit. Without it #\31b would be parsed as a 3-character escape (\31b) instead of \31 followed by b.

aristotelos · 2025-12-17T05:52:21Z

 {
 public class ConditionParserTests
 {
- [TestCase("[name=\"2\"]")]


Why were the ParseCondition_ByCssAttributeName_ReturnsCondition test cases removed?

The old test was removed because it was testing the same functionality now covered by ParseCondition_CssSelectorWithEscapedSpecialChars_ReturnsNameCondition. Should I keep both for coverage?

aristotelos · 2025-12-17T05:56:17Z

Please squash the commits in this merge request together.

E.g. with git rebase -i or with:

git reset main
git commit

sinpru · 2025-11-13T04:17:15Z

Fixes#138

aristotelos · 2025-12-06T11:06:58Z

+
+ private (string strategy, string value) ParseCssSelector(string cssSelector)
+ {
+ if (cssSelector.StartsWith("#"))


This is not sufficient to judge that it is a simple CSS ID selector. To be sure, shouldn't a regex be used that checks that the complete cssSelector string matches a simple ID selector? It may be that the user sends a compound selector instead, and we should have a proper error message in that case that this is not supported.
The same comment holds for all parses below. To check that a regex matches the string fully, start with ^ and end with $.

aristotelos · 2025-12-08T05:36:13Z

Would it be possible to add a unit test to e.g. ConditionParser to check the case that is currently not working?

sinpru · 2025-12-14T15:39:20Z

Actually I found out that my approach to fixing the CSS selector inside the controller was a mistake. I've refactored this to handle CSS selector parsing in ConditionParser instead.

aristotelos · 2025-12-08T05:25:15Z

- }
-
- var nameMatch = Regex.Match(cssSelector, @"\*?\[name\s*=\s*""(.+?)""\]", RegexOptions.IgnoreCase);
+ var nameMatch = Regex.Match(cssSelector, @"^\*?\[name\s*=\s*""(.+?)""\]$", RegexOptions.IgnoreCase);


Couldn't this still match [name="test"][class="test2"] because of the .+ expression? If so, I suggest [^""]+ instead. Please add a unit test to validate that [name="test"][class="test2"] throws as not supported.

You're right, the .+ is too greedy, I've added a unit test that validates compound selectors throws as intened.

aristotelos · 2025-12-08T05:35:05Z

 { 
 element = await Wait.Until(() => startNode().FindFirstByXPath(findElementRequest.Value), element => element != null, session.ImplicitWaitTimeout);
 }
+ else if (findElementRequest.Using == "css selector")


I've had a closer look and it seems that the ConditionParser should already allow escaped characters too. Instead of adding another case here, can't we fix the ConditionParser regular expressions if they fail?

aristotelos · 2025-12-17T05:48:08Z

@@ -31,19 +29,24 @@ public class ConditionParser : IConditionParser
 /// - Attribute equals attribute (e.g. `[name=value]`) not supported.


Also update the comment above Unicode escape characters or escape characters in the attribute name are not supported. -> Escape characters in the attribute name are not supported.

aristotelos · 2025-12-17T05:49:14Z

 /// - Multiple selectors are not supported.
 /// </summary>
- private static Regex SimpleCssIdSelectorRegex = new Regex(@"^#(?<name>(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f]))+)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssIdSelectorRegex = new Regex(@"^#(?<name>(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))+)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

The \s? is needed to make the trailing space optional per CSS spec. Unicode escapes like \34 can be followed by an optional space (\34 b or \34b are both valid depending on context).

aristotelos · 2025-12-17T05:49:21Z

 /// - Multiple selectors are not supported.
 /// </summary>
- private static Regex SimpleCssClassSelectorRegex = new Regex(@"^\.(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f]))*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssClassSelectorRegex = new Regex(@"^\.(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

This goes the same as the one above

aristotelos · 2025-12-17T05:49:27Z

 /// - ~= or |= not supported.
 /// </summary>
- private static Regex SimpleCssAttributeSelectorRegex = new Regex(@"^\*?\[\s*(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377])*)\s*=\s*(?<string>(?<string1>""(?<string1value>([^\n\r\f\\""]|(?<escape>\\[^\r\n\f0-9a-f]))*)"")|(?<string2>'(?<string2value>([^\n\r\f\\']|(?<escape>\\[^\r\n\f0-9a-f]))*)'))\s*\]$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssAttributeSelectorRegex = new Regex(@"^\*?\[\s*(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377])*)\s*=\s*(?<string>(?<string1>""(?<string1value>([^\n\r\f\\""]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)"")|(?<string2>'(?<string2value>([^\n\r\f\\']|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)'))\s*\]$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

aristotelos · 2025-12-17T05:50:39Z

+ [Test]
+ public void ParseCondition_CssSelectorWithSimpleNumericId_ReturnsAutomationIdCondition()
+ {
+ var cssSelector = @"#\31 ";


Isn't "#\31" without the trailing space a better example for this case?

The trailing space is needed when the next character is a hex digit. Without it #\31b would be parsed as a 3-character escape (\31b) instead of \31 followed by b.

aristotelos · 2025-12-17T05:52:21Z

 {
 public class ConditionParserTests
 {
- [TestCase("[name=\"2\"]")]


Why were the ParseCondition_ByCssAttributeName_ReturnsCondition test cases removed?

The old test was removed because it was testing the same functionality now covered by ParseCondition_CssSelectorWithEscapedSpecialChars_ReturnsNameCondition. Should I keep both for coverage?

aristotelos · 2025-12-17T05:56:17Z

Please squash the commits in this merge request together.

E.g. with git rebase -i or with:

git reset main
git commit

sinpru · 2025-11-13T04:17:15Z

Fixes#138

aristotelos · 2025-12-06T11:06:58Z

+
+ private (string strategy, string value) ParseCssSelector(string cssSelector)
+ {
+ if (cssSelector.StartsWith("#"))


This is not sufficient to judge that it is a simple CSS ID selector. To be sure, shouldn't a regex be used that checks that the complete cssSelector string matches a simple ID selector? It may be that the user sends a compound selector instead, and we should have a proper error message in that case that this is not supported.
The same comment holds for all parses below. To check that a regex matches the string fully, start with ^ and end with $.

aristotelos · 2025-12-08T05:36:13Z

Would it be possible to add a unit test to e.g. ConditionParser to check the case that is currently not working?

sinpru · 2025-12-14T15:39:20Z

Actually I found out that my approach to fixing the CSS selector inside the controller was a mistake. I've refactored this to handle CSS selector parsing in ConditionParser instead.

aristotelos · 2025-12-08T05:25:15Z

- }
-
- var nameMatch = Regex.Match(cssSelector, @"\*?\[name\s*=\s*""(.+?)""\]", RegexOptions.IgnoreCase);
+ var nameMatch = Regex.Match(cssSelector, @"^\*?\[name\s*=\s*""(.+?)""\]$", RegexOptions.IgnoreCase);


Couldn't this still match [name="test"][class="test2"] because of the .+ expression? If so, I suggest [^""]+ instead. Please add a unit test to validate that [name="test"][class="test2"] throws as not supported.

You're right, the .+ is too greedy, I've added a unit test that validates compound selectors throws as intened.

aristotelos · 2025-12-08T05:35:05Z

 { 
 element = await Wait.Until(() => startNode().FindFirstByXPath(findElementRequest.Value), element => element != null, session.ImplicitWaitTimeout);
 }
+ else if (findElementRequest.Using == "css selector")


I've had a closer look and it seems that the ConditionParser should already allow escaped characters too. Instead of adding another case here, can't we fix the ConditionParser regular expressions if they fail?

aristotelos · 2025-12-17T05:48:08Z

@@ -31,19 +29,24 @@ public class ConditionParser : IConditionParser
 /// - Attribute equals attribute (e.g. `[name=value]`) not supported.


Also update the comment above Unicode escape characters or escape characters in the attribute name are not supported. -> Escape characters in the attribute name are not supported.

aristotelos · 2025-12-17T05:49:14Z

 /// - Multiple selectors are not supported.
 /// </summary>
- private static Regex SimpleCssIdSelectorRegex = new Regex(@"^#(?<name>(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f]))+)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssIdSelectorRegex = new Regex(@"^#(?<name>(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))+)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

The \s? is needed to make the trailing space optional per CSS spec. Unicode escapes like \34 can be followed by an optional space (\34 b or \34b are both valid depending on context).

aristotelos · 2025-12-17T05:49:21Z

 /// - Multiple selectors are not supported.
 /// </summary>
- private static Regex SimpleCssClassSelectorRegex = new Regex(@"^\.(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f]))*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssClassSelectorRegex = new Regex(@"^\.(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

This goes the same as the one above

aristotelos · 2025-12-17T05:49:27Z

 /// - ~= or |= not supported.
 /// </summary>
- private static Regex SimpleCssAttributeSelectorRegex = new Regex(@"^\*?\[\s*(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377])*)\s*=\s*(?<string>(?<string1>""(?<string1value>([^\n\r\f\\""]|(?<escape>\\[^\r\n\f0-9a-f]))*)"")|(?<string2>'(?<string2value>([^\n\r\f\\']|(?<escape>\\[^\r\n\f0-9a-f]))*)'))\s*\]$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssAttributeSelectorRegex = new Regex(@"^\*?\[\s*(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377])*)\s*=\s*(?<string>(?<string1>""(?<string1value>([^\n\r\f\\""]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)"")|(?<string2>'(?<string2value>([^\n\r\f\\']|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)'))\s*\]$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

aristotelos · 2025-12-17T05:50:39Z

+ [Test]
+ public void ParseCondition_CssSelectorWithSimpleNumericId_ReturnsAutomationIdCondition()
+ {
+ var cssSelector = @"#\31 ";


Isn't "#\31" without the trailing space a better example for this case?

The trailing space is needed when the next character is a hex digit. Without it #\31b would be parsed as a 3-character escape (\31b) instead of \31 followed by b.

aristotelos · 2025-12-17T05:52:21Z

 {
 public class ConditionParserTests
 {
- [TestCase("[name=\"2\"]")]


Why were the ParseCondition_ByCssAttributeName_ReturnsCondition test cases removed?

The old test was removed because it was testing the same functionality now covered by ParseCondition_CssSelectorWithEscapedSpecialChars_ReturnsNameCondition. Should I keep both for coverage?

aristotelos · 2025-12-17T05:56:17Z

Please squash the commits in this merge request together.

E.g. with git rebase -i or with:

git reset main
git commit

sinpru · 2025-11-13T04:17:15Z

Fixes#138

aristotelos · 2025-12-06T11:06:58Z

+
+ private (string strategy, string value) ParseCssSelector(string cssSelector)
+ {
+ if (cssSelector.StartsWith("#"))


This is not sufficient to judge that it is a simple CSS ID selector. To be sure, shouldn't a regex be used that checks that the complete cssSelector string matches a simple ID selector? It may be that the user sends a compound selector instead, and we should have a proper error message in that case that this is not supported.
The same comment holds for all parses below. To check that a regex matches the string fully, start with ^ and end with $.

aristotelos · 2025-12-08T05:36:13Z

Would it be possible to add a unit test to e.g. ConditionParser to check the case that is currently not working?

sinpru · 2025-12-14T15:39:20Z

Actually I found out that my approach to fixing the CSS selector inside the controller was a mistake. I've refactored this to handle CSS selector parsing in ConditionParser instead.

aristotelos · 2025-12-08T05:25:15Z

- }
-
- var nameMatch = Regex.Match(cssSelector, @"\*?\[name\s*=\s*""(.+?)""\]", RegexOptions.IgnoreCase);
+ var nameMatch = Regex.Match(cssSelector, @"^\*?\[name\s*=\s*""(.+?)""\]$", RegexOptions.IgnoreCase);


Couldn't this still match [name="test"][class="test2"] because of the .+ expression? If so, I suggest [^""]+ instead. Please add a unit test to validate that [name="test"][class="test2"] throws as not supported.

You're right, the .+ is too greedy, I've added a unit test that validates compound selectors throws as intened.

aristotelos · 2025-12-08T05:35:05Z

 { 
 element = await Wait.Until(() => startNode().FindFirstByXPath(findElementRequest.Value), element => element != null, session.ImplicitWaitTimeout);
 }
+ else if (findElementRequest.Using == "css selector")


I've had a closer look and it seems that the ConditionParser should already allow escaped characters too. Instead of adding another case here, can't we fix the ConditionParser regular expressions if they fail?

aristotelos · 2025-12-17T05:48:08Z

@@ -31,19 +29,24 @@ public class ConditionParser : IConditionParser
 /// - Attribute equals attribute (e.g. `[name=value]`) not supported.


Also update the comment above Unicode escape characters or escape characters in the attribute name are not supported. -> Escape characters in the attribute name are not supported.

aristotelos · 2025-12-17T05:49:14Z

 /// - Multiple selectors are not supported.
 /// </summary>
- private static Regex SimpleCssIdSelectorRegex = new Regex(@"^#(?<name>(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f]))+)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssIdSelectorRegex = new Regex(@"^#(?<name>(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))+)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

The \s? is needed to make the trailing space optional per CSS spec. Unicode escapes like \34 can be followed by an optional space (\34 b or \34b are both valid depending on context).

aristotelos · 2025-12-17T05:49:21Z

 /// - Multiple selectors are not supported.
 /// </summary>
- private static Regex SimpleCssClassSelectorRegex = new Regex(@"^\.(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f]))*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssClassSelectorRegex = new Regex(@"^\.(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

This goes the same as the one above

aristotelos · 2025-12-17T05:49:27Z

 /// - ~= or |= not supported.
 /// </summary>
- private static Regex SimpleCssAttributeSelectorRegex = new Regex(@"^\*?\[\s*(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377])*)\s*=\s*(?<string>(?<string1>""(?<string1value>([^\n\r\f\\""]|(?<escape>\\[^\r\n\f0-9a-f]))*)"")|(?<string2>'(?<string2value>([^\n\r\f\\']|(?<escape>\\[^\r\n\f0-9a-f]))*)'))\s*\]$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssAttributeSelectorRegex = new Regex(@"^\*?\[\s*(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377])*)\s*=\s*(?<string>(?<string1>""(?<string1value>([^\n\r\f\\""]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)"")|(?<string2>'(?<string2value>([^\n\r\f\\']|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)'))\s*\]$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

aristotelos · 2025-12-17T05:50:39Z

+ [Test]
+ public void ParseCondition_CssSelectorWithSimpleNumericId_ReturnsAutomationIdCondition()
+ {
+ var cssSelector = @"#\31 ";


Isn't "#\31" without the trailing space a better example for this case?

The trailing space is needed when the next character is a hex digit. Without it #\31b would be parsed as a 3-character escape (\31b) instead of \31 followed by b.

aristotelos · 2025-12-17T05:52:21Z

 {
 public class ConditionParserTests
 {
- [TestCase("[name=\"2\"]")]


Why were the ParseCondition_ByCssAttributeName_ReturnsCondition test cases removed?

The old test was removed because it was testing the same functionality now covered by ParseCondition_CssSelectorWithEscapedSpecialChars_ReturnsNameCondition. Should I keep both for coverage?

aristotelos · 2025-12-17T05:56:17Z

Please squash the commits in this merge request together.

E.g. with git rebase -i or with:

git reset main
git commit

sinpru · 2025-11-13T04:17:15Z

Fixes#138

aristotelos · 2025-12-06T11:06:58Z

+
+ private (string strategy, string value) ParseCssSelector(string cssSelector)
+ {
+ if (cssSelector.StartsWith("#"))


This is not sufficient to judge that it is a simple CSS ID selector. To be sure, shouldn't a regex be used that checks that the complete cssSelector string matches a simple ID selector? It may be that the user sends a compound selector instead, and we should have a proper error message in that case that this is not supported.
The same comment holds for all parses below. To check that a regex matches the string fully, start with ^ and end with $.

aristotelos · 2025-12-08T05:36:13Z

Would it be possible to add a unit test to e.g. ConditionParser to check the case that is currently not working?

sinpru · 2025-12-14T15:39:20Z

Actually I found out that my approach to fixing the CSS selector inside the controller was a mistake. I've refactored this to handle CSS selector parsing in ConditionParser instead.

aristotelos · 2025-12-08T05:25:15Z

- }
-
- var nameMatch = Regex.Match(cssSelector, @"\*?\[name\s*=\s*""(.+?)""\]", RegexOptions.IgnoreCase);
+ var nameMatch = Regex.Match(cssSelector, @"^\*?\[name\s*=\s*""(.+?)""\]$", RegexOptions.IgnoreCase);


Couldn't this still match [name="test"][class="test2"] because of the .+ expression? If so, I suggest [^""]+ instead. Please add a unit test to validate that [name="test"][class="test2"] throws as not supported.

You're right, the .+ is too greedy, I've added a unit test that validates compound selectors throws as intened.

aristotelos · 2025-12-08T05:35:05Z

 { 
 element = await Wait.Until(() => startNode().FindFirstByXPath(findElementRequest.Value), element => element != null, session.ImplicitWaitTimeout);
 }
+ else if (findElementRequest.Using == "css selector")


I've had a closer look and it seems that the ConditionParser should already allow escaped characters too. Instead of adding another case here, can't we fix the ConditionParser regular expressions if they fail?

aristotelos · 2025-12-17T05:48:08Z

@@ -31,19 +29,24 @@ public class ConditionParser : IConditionParser
 /// - Attribute equals attribute (e.g. `[name=value]`) not supported.


Also update the comment above Unicode escape characters or escape characters in the attribute name are not supported. -> Escape characters in the attribute name are not supported.

aristotelos · 2025-12-17T05:49:14Z

 /// - Multiple selectors are not supported.
 /// </summary>
- private static Regex SimpleCssIdSelectorRegex = new Regex(@"^#(?<name>(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f]))+)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssIdSelectorRegex = new Regex(@"^#(?<name>(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))+)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

The \s? is needed to make the trailing space optional per CSS spec. Unicode escapes like \34 can be followed by an optional space (\34 b or \34b are both valid depending on context).

aristotelos · 2025-12-17T05:49:21Z

 /// - Multiple selectors are not supported.
 /// </summary>
- private static Regex SimpleCssClassSelectorRegex = new Regex(@"^\.(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f]))*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssClassSelectorRegex = new Regex(@"^\.(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

This goes the same as the one above

aristotelos · 2025-12-17T05:49:27Z

 /// - ~= or |= not supported.
 /// </summary>
- private static Regex SimpleCssAttributeSelectorRegex = new Regex(@"^\*?\[\s*(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377])*)\s*=\s*(?<string>(?<string1>""(?<string1value>([^\n\r\f\\""]|(?<escape>\\[^\r\n\f0-9a-f]))*)"")|(?<string2>'(?<string2value>([^\n\r\f\\']|(?<escape>\\[^\r\n\f0-9a-f]))*)'))\s*\]$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssAttributeSelectorRegex = new Regex(@"^\*?\[\s*(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377])*)\s*=\s*(?<string>(?<string1>""(?<string1value>([^\n\r\f\\""]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)"")|(?<string2>'(?<string2value>([^\n\r\f\\']|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)'))\s*\]$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

aristotelos · 2025-12-17T05:50:39Z

+ [Test]
+ public void ParseCondition_CssSelectorWithSimpleNumericId_ReturnsAutomationIdCondition()
+ {
+ var cssSelector = @"#\31 ";


Isn't "#\31" without the trailing space a better example for this case?

The trailing space is needed when the next character is a hex digit. Without it #\31b would be parsed as a 3-character escape (\31b) instead of \31 followed by b.

aristotelos · 2025-12-17T05:52:21Z

 {
 public class ConditionParserTests
 {
- [TestCase("[name=\"2\"]")]


Why were the ParseCondition_ByCssAttributeName_ReturnsCondition test cases removed?

The old test was removed because it was testing the same functionality now covered by ParseCondition_CssSelectorWithEscapedSpecialChars_ReturnsNameCondition. Should I keep both for coverage?

aristotelos · 2025-12-17T05:56:17Z

Please squash the commits in this merge request together.

E.g. with git rebase -i or with:

git reset main
git commit

sinpru · 2025-11-13T04:17:15Z

Fixes#138

aristotelos · 2025-12-06T11:06:58Z

+
+ private (string strategy, string value) ParseCssSelector(string cssSelector)
+ {
+ if (cssSelector.StartsWith("#"))


This is not sufficient to judge that it is a simple CSS ID selector. To be sure, shouldn't a regex be used that checks that the complete cssSelector string matches a simple ID selector? It may be that the user sends a compound selector instead, and we should have a proper error message in that case that this is not supported.
The same comment holds for all parses below. To check that a regex matches the string fully, start with ^ and end with $.

aristotelos · 2025-12-08T05:36:13Z

Would it be possible to add a unit test to e.g. ConditionParser to check the case that is currently not working?

sinpru · 2025-12-14T15:39:20Z

Actually I found out that my approach to fixing the CSS selector inside the controller was a mistake. I've refactored this to handle CSS selector parsing in ConditionParser instead.

aristotelos · 2025-12-08T05:25:15Z

- }
-
- var nameMatch = Regex.Match(cssSelector, @"\*?\[name\s*=\s*""(.+?)""\]", RegexOptions.IgnoreCase);
+ var nameMatch = Regex.Match(cssSelector, @"^\*?\[name\s*=\s*""(.+?)""\]$", RegexOptions.IgnoreCase);


Couldn't this still match [name="test"][class="test2"] because of the .+ expression? If so, I suggest [^""]+ instead. Please add a unit test to validate that [name="test"][class="test2"] throws as not supported.

You're right, the .+ is too greedy, I've added a unit test that validates compound selectors throws as intened.

aristotelos · 2025-12-08T05:35:05Z

 { 
 element = await Wait.Until(() => startNode().FindFirstByXPath(findElementRequest.Value), element => element != null, session.ImplicitWaitTimeout);
 }
+ else if (findElementRequest.Using == "css selector")


I've had a closer look and it seems that the ConditionParser should already allow escaped characters too. Instead of adding another case here, can't we fix the ConditionParser regular expressions if they fail?

aristotelos · 2025-12-17T05:48:08Z

@@ -31,19 +29,24 @@ public class ConditionParser : IConditionParser
 /// - Attribute equals attribute (e.g. `[name=value]`) not supported.


Also update the comment above Unicode escape characters or escape characters in the attribute name are not supported. -> Escape characters in the attribute name are not supported.

aristotelos · 2025-12-17T05:49:14Z

 /// - Multiple selectors are not supported.
 /// </summary>
- private static Regex SimpleCssIdSelectorRegex = new Regex(@"^#(?<name>(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f]))+)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssIdSelectorRegex = new Regex(@"^#(?<name>(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))+)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

The \s? is needed to make the trailing space optional per CSS spec. Unicode escapes like \34 can be followed by an optional space (\34 b or \34b are both valid depending on context).

aristotelos · 2025-12-17T05:49:21Z

 /// - Multiple selectors are not supported.
 /// </summary>
- private static Regex SimpleCssClassSelectorRegex = new Regex(@"^\.(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f]))*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssClassSelectorRegex = new Regex(@"^\.(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

This goes the same as the one above

aristotelos · 2025-12-17T05:49:27Z

 /// - ~= or |= not supported.
 /// </summary>
- private static Regex SimpleCssAttributeSelectorRegex = new Regex(@"^\*?\[\s*(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377])*)\s*=\s*(?<string>(?<string1>""(?<string1value>([^\n\r\f\\""]|(?<escape>\\[^\r\n\f0-9a-f]))*)"")|(?<string2>'(?<string2value>([^\n\r\f\\']|(?<escape>\\[^\r\n\f0-9a-f]))*)'))\s*\]$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+ private static Regex SimpleCssAttributeSelectorRegex = new Regex(@"^\*?\[\s*(?<ident>-?(?<nmstart>[_a-z]|[\240-\377])(?<nmchar>[_a-z0-9-]|[\240-\377])*)\s*=\s*(?<string>(?<string1>""(?<string1value>([^\n\r\f\\""]|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)"")|(?<string2>'(?<string2value>([^\n\r\f\\']|(?<escape>\\[^\r\n\f0-9a-f])|(?<unicode>\\[0-9a-fA-F]{1,6}\s?))*)'))\s*\]$", RegexOptions.Compiled | RegexOptions.IgnoreCase);


Shouldn't the \s? part be omitted here?

aristotelos · 2025-12-17T05:50:39Z

+ [Test]
+ public void ParseCondition_CssSelectorWithSimpleNumericId_ReturnsAutomationIdCondition()
+ {
+ var cssSelector = @"#\31 ";


Isn't "#\31" without the trailing space a better example for this case?

The trailing space is needed when the next character is a hex digit. Without it #\31b would be parsed as a 3-character escape (\31b) instead of \31 followed by b.

aristotelos · 2025-12-17T05:52:21Z

 {
 public class ConditionParserTests
 {
- [TestCase("[name=\"2\"]")]


Why were the ParseCondition_ByCssAttributeName_ReturnsCondition test cases removed?

The old test was removed because it was testing the same functionality now covered by ParseCondition_CssSelectorWithEscapedSpecialChars_ReturnsNameCondition. Should I keep both for coverage?

aristotelos · 2025-12-17T05:56:17Z

Please squash the commits in this merge request together.

E.g. with git rebase -i or with:

git reset main
git commit

aristotelos reviewed Dec 6, 2025
View reviewed changes

aristotelos requested changes Dec 17, 2025
View reviewed changes

Fix CSS selector support for numeric IDs and escape sequences
81ad120

sinpru force-pushed the fix-finding-element-with-number branch from bbc21dd to 81ad120Compare January 7, 2026 04:22

aristotelos reviewed Dec 6, 2025
View reviewed changes

aristotelos requested changes Dec 17, 2025
View reviewed changes

Fix CSS selector support for numeric IDs and escape sequences
81ad120

sinpru force-pushed the fix-finding-element-with-number branch from bbc21dd to 81ad120Compare January 7, 2026 04:22

aristotelos reviewed Dec 6, 2025
View reviewed changes

aristotelos requested changes Dec 17, 2025
View reviewed changes

Fix CSS selector support for numeric IDs and escape sequences
81ad120

sinpru force-pushed the fix-finding-element-with-number branch from bbc21dd to 81ad120Compare January 7, 2026 04:22

aristotelos reviewed Dec 6, 2025
View reviewed changes

aristotelos requested changes Dec 17, 2025
View reviewed changes

Fix CSS selector support for numeric IDs and escape sequences
81ad120

sinpru force-pushed the fix-finding-element-with-number branch from bbc21dd to 81ad120Compare January 7, 2026 04:22

aristotelos reviewed Dec 6, 2025
View reviewed changes

aristotelos requested changes Dec 17, 2025
View reviewed changes

Fix CSS selector support for numeric IDs and escape sequences
81ad120

sinpru force-pushed the fix-finding-element-with-number branch from bbc21dd to 81ad120Compare January 7, 2026 04:22

aristotelos reviewed Dec 6, 2025
View reviewed changes

aristotelos requested changes Dec 17, 2025
View reviewed changes

Fix CSS selector support for numeric IDs and escape sequences
81ad120

sinpru force-pushed the fix-finding-element-with-number branch from bbc21dd to 81ad120Compare January 7, 2026 04:22

aristotelos reviewed Dec 6, 2025
View reviewed changes

aristotelos requested changes Dec 17, 2025
View reviewed changes

Fix CSS selector support for numeric IDs and escape sequences
81ad120

sinpru force-pushed the fix-finding-element-with-number branch from bbc21dd to 81ad120Compare January 7, 2026 04:22

aristotelos reviewed Dec 6, 2025
View reviewed changes

aristotelos requested changes Dec 17, 2025
View reviewed changes

Fix CSS selector support for numeric IDs and escape sequences
81ad120

sinpru force-pushed the fix-finding-element-with-number branch from bbc21dd to 81ad120Compare January 7, 2026 04:22

		@@ -31,19 +29,24 @@ public class ConditionParser : IConditionParser
		/// - Attribute equals attribute (e.g. `[name=value]`) not supported.

Uh oh!

Conversation

sinpru commented Nov 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aristotelos commented Dec 8, 2025

Uh oh!

sinpru commented Dec 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sinpruJan 7, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aristotelos commented Dec 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

sinpru commented Nov 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aristotelos commented Dec 8, 2025

Uh oh!

sinpru commented Dec 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sinpruJan 7, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sinpruJan 7, 2026•
edited
Loading

sinpruJan 7, 2026•
edited
Loading