Repository navigation
feat(locator): add a context-wide visibility default - #43859
Conversation
This comment has been minimized.
This comment has been minimized.
|
Make it |
|
Hi, I'm the Playwright bot. I started to address the review comments on Linux: agent run |
🟢 Both failures are pre-existing flakesHi, I'm the Playwright bot. I checked the failed jobs, the PR changes, and refreshed CI history. Neither failure looks caused by this PR. DetailsThe two failures in the latest merged report, for PR commit Pre-existing flake / infra
CI failures checked by the Playwright bot - agent run Triaged by the Playwright bot - agent run |
|
Hi, I'm the Playwright bot and I revised the option names and defaults. Pavel Feldman (@pavelfeldman): Changed the context/test option to Verified on Linux: 171 context/locator tests across Chromium, Firefox and WebKit, 64 parser/generator tests, and four configuration tests passed. Both regression tests failed without the implementation in all three browsers. Build and Revised on Linux by the Playwright bot - agent run |
This comment has been minimized.
This comment has been minimized.
Test results for "MCP"5194 passed, 616 skipped Merge workflow run. |
Test results for "tests 1"23588 passed, 705 skipped Merge workflow run. |
|
|
||
| if (options?.visible !== undefined) | ||
| this._selector += ` >> visible=${options.visible ? 'true' : 'false'}`; | ||
| this._selectorWithoutVisibility += ` >> visible=${options.visible ? 'true' : 'false'}`; |
There was a problem hiding this comment.
We are adding visibility here whereas the field is _selectorWithoutVisibility
There was a problem hiding this comment.
It is correct here. When visible is requested on this level, it should be baked into the locator's selectorWithoutVisibility. Only the last token's visible is dynamic.
| (this as any)[inspect.custom] = () => this._inspect(); | ||
| } | ||
|
|
||
| get _selector(): string { |
There was a problem hiding this comment.
Can we make it a method for clarity?
There was a problem hiding this comment.
The world will break.
|
Wow, that's it? Is it really done? In a few hours from the issue created? Thanks 👏 Do you have a regular release or it is more an irregular batch of features? Can we really expect this in the next version? |
Add the opt-in
locatorIncludeHidden: falsecontext/test option and per-locatorincludeHiddenoverrides. Keep the unfiltered selector separate so automatic visibility filtering applies only when resolving the final locator.Overrides survive same-element transformations; descendant locators use the context default again. Existing role accessibility rules and the default matching behavior remain unchanged.
A separate context test is needed because the existing strict-selector and page visibility tests do not configure this context default. Existing visibility and configuration tests are extended.
Verified on Linux: 275 context/locator tests, 64 parser/generator tests and 6 configuration tests passed; one test was skipped. Both regression tests fail without automatic filtering in all three browsers. Build and
flintpassed.Fixes #43855
Triaged on Linux by the Playwright bot - agent run