Fix CSS selector support for numeric IDs and escape sequences #150

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

Fixes #138

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

src/FlaUI.WebDriver/Controllers/FindElementsController.cs

+
+        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

src/FlaUI.WebDriver/Controllers/FindElementsController.cs

-            }
-
-            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

src/FlaUI.WebDriver/Controllers/FindElementsController.cs

            { 
                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

src/FlaUI.WebDriver/Services/ConditionParser.cs

        /// - Unicode escape characters or escape characters in the attribute name are not supported.
        /// - Multiple selectors are not supported.
        /// - Attribute presence selector (e.g. `[name]`) not supported.
        /// - 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

src/FlaUI.WebDriver/Services/ConditionParser.cs

        /// - 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

src/FlaUI.WebDriver/Services/ConditionParser.cs

        /// - 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

src/FlaUI.WebDriver/Services/ConditionParser.cs

        /// - ~= 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

src/FlaUI.WebDriver.UnitTests/Services/ConditionParserTests.cs

+        [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

src/FlaUI.WebDriver.UnitTests/Services/ConditionParserTests.cs

 {
    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 81ad120 Compare January 7, 2026 04:22

Fix CSS selector support for numeric IDs and escape sequences #150

Are you sure you want to change the base?

Fix CSS selector support for numeric IDs and escape sequences #150

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!

sinpru Jan 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

sinpru Jan 7, 2026 •

edited

Loading