Skip to content

fix(selectors): reset regex state before matching each element - #42818

Merged
Yury Semikhatsky (yury-s) merged 3 commits into
microsoft:mainfrom
Abnoz01:fix-42813
Sep 22, 2026
Merged

Yury Semikhatsky (yury-s) merged 3 commits into
microsoft:mainfrom
Abnoz01:fix-42813

Conversation

@Abnoz01

Copy link
Copy Markdown
Contributor

Summary

  • getByText, getByLabel and the text-matches engine reuse a single RegExp across elements. With the g or y flag, RegExp.prototype.test carries lastIndex between calls, so every other match was skipped (getByText(/foo/g) found 4 of 6 elements).
  • Reset lastIndex before each test, also in the expect text matcher, and add a test.

Fixes #42813

Text and label matchers reuse one RegExp across elements. With the g or
y flag, RegExp.prototype.test keeps lastIndex between calls, so every
other matching element was skipped.

Fixes microsoft#42813
@yury-s

Copy link
Copy Markdown
Member

Approach looks right, thanks. Two more sites reuse the regex across elements and are still stateful for y:

  • createAttributeMatcher in injectedScript.ts (getByPlaceholder, getByAltText, getByTitle, getByTestId)
  • matchesAttributePart in selectorUtils.ts (getByRole name/description)

They use String.prototype.match, which resets lastIndex for g but not for y, so getByRole('button', { name: /foo/y }) still finds 2 of 3.

Please cover those too and add getByRole and getByPlaceholder with /foo/y to the test.

String.prototype.match resets lastIndex for the g flag but not for y,
so getByRole name matching and attribute-based locators were still
stateful across elements.
@Abnoz01

Copy link
Copy Markdown
Contributor Author

Thanks, good catch. Pushed a follow-up that resets lastIndex in createAttributeMatcher and matchesAttributePart as well, and extended the test with getByRole and getByPlaceholder using /foo/g and /foo/y. Confirmed the getByRole('button', { name: /foo/y }) case fails without the change and passes with it.

@yury-s Yury Semikhatsky (yury-s) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Drop the comments, they don't add anything.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

8 flaky ⚠️ [chromium-library] › library/video.spec.ts:690 › screencast › should capture full viewport `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/browsercontext-page-event.spec.ts:173 › should work with Ctrl-clicking `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/chromium/chromium.spec.ts:436 › should produce network events, routing, and annotations for Service Worker (advanced) `@chromium-ubuntu-22.04-node24`
⚠️ [chromium-library] › library/video.spec.ts:762 › screencast › should work with video+trace `@chromium-ubuntu-22.04-node20`
⚠️ [firefox-library] › library/browsercontext-cookies-third-party.spec.ts:257 › third party 'Partitioned;' cookies `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-library] › library/browsercontext-cookies-third-party.spec.ts:470 › top level 'Partitioned;' cookie and same origin iframe `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-page] › page/page-network-response.spec.ts:363 › should return body for prefetch script `@firefox-ubuntu-22.04-node20`
⚠️ [webkit-page] › page/page-set-input-files.spec.ts:38 › should upload a folder `@webkit-ubuntu-22.04-node20`

52085 passed, 1252 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

2 failed
❌ [chromium] › mcp/cli-webmcp.spec.ts:162 › webmcp-call disambiguates duplicate tool names by frame @mcp-macos-latest-chromium
❌ [firefox] › mcp/cli-session.spec.ts:54 › idle timeout shuts the session down @mcp-windows-latest-firefox

8690 passed, 1474 skipped


Merge workflow run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: getByText/getByLabel with a global or sticky regex (/x/g, /x/y) match only some of the elements

2 participants