core: match item labels exactly in item-within-container lookups (#149) - #182
Merged
Merged
Conversation
Playwright's setName, getByText and setHasText all default to a case-insensitive substring match, so a lookup for "Option 1" also matched "Option 10", and the trailing .first() resolved that ambiguity silently in DOM order. For a test library that is worse than a misdirected click: the assertion re-runs the same lookup, so the test goes green against the wrong element. Item-within-container lookups — one option out of a combo box, select, list box or checkbox/radio group, one tab, one menu item, one side-nav item, one accordion panel, one upload row — now match the item's whole label and drop .first(), so a genuine duplicate raises a Playwright strict-mode error. Two new helpers carry the convention so element authors don't hand-roll the chain: AccessibleNameLocator.findExact for role + exact accessible name, and ItemLocator for exact-text lookups. ItemLocator.byOwnExactText matches only an item's own text nodes, which is what vaadin-side-nav-item needs — matching the subtree would read a parent as "Parent Child 1 Child 2" and never find it by its own label. HasCheckedElement.getInputLocator() loses its .first() too: a checkbox holds exactly one input, so the only ambiguity it could hide was an ambiguous component locator. Page-level getByLabel(Page, String) field factories, AccessibleNameLocator.find and the helpers documented as "contains" (Badge, Notification, Tooltip, VirtualList.getItemByText) keep substring matching. That is now a documented decision rather than a Playwright default that leaked through, spelled out in AGENTS.md, START-HERE.md and each Javadoc. Two consequences for callers: exact matches are case-sensitive, and a partial label no longer resolves. ListBoxViewIT relied on both and is updated. Fixes #149 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #149.
Problem
Playwright's
setName,getByTextandsetHasTextall default to a case-insensitive substring match. Every label-keyed lookup in the library used one of them and closed with.first(), so an ambiguity was resolved silently in DOM order. For a test library that is worse than a misdirected click: the assertion helper re-runs the same lookup, so the test goes green against the wrong element.With items in DOM order
["Option 10", "Option 1"]:The convention
.first()→ a genuine duplicate raises a Playwright strict-mode errorgetByLabel(Page, …)field factories,AccessibleNameLocator.find,getByTexthelpers documented as "contains".first(), kept for convenience — now a documented decision rather than a Playwright default that leaked throughMessageListElement.getMessageByUserNameis the one documented exception among item lookups: it matches the author exactly but keeps.first(), because several messages from one author is normal data, not an ambiguity.Helpers (issue item 2)
So element authors don't hand-roll the chain:
AccessibleNameLocator.findExact(Page|Locator, tag, role, label)— exact accessible name, no placeholder fallback, no.first().ItemLocator—byExactName(role + exact accessible name),byExactText(the item or an element inside it),byOwnExactText(the item's own text nodes only),exactText.byOwnExactTextexists forvaadin-side-nav-item: a nested item's subtree text reads"Section Section 1 Section 2", so whole-subtree matching can never find a parent by its own label.:text-is()matches immediate text nodes, which is exactly what Flow'sSideNavItem.setLabelrenders (Element.createText).Converted to exact matching
CheckboxElement.getByLabel(Locator, …),RadioButtonElement.getByLabel,TabsElement.getTab(String)(soTabSheetElement.getTab/selectTabtoo),TabElement.getTabByText,MenuItemElement.getByLabel(soMenuBarElement/MenuElement),ContextMenuElementitems,ComboBoxElement/MultiSelectComboBoxElement/SelectElement/ListBoxElementoverlay items,SideNavigationElement.getItem,AccordionPanelElement.getAccordionPanelBySummary,UploadElement.getFileItemLocator,MessageListElement.getMessageByUserName.MarkdownElement.getLinkandBreadcrumbsElement.getItemwere already exact and are unchanged.HasCheckedElement.getInputLocator()also loses its.first(): a checkbox-like component holds exactly one input, so the only ambiguity it could hide was an ambiguous component locator — which would otherwise re-mask the strict-mode error one level down.Breaking changes for callers
Two, both intended:
ListBoxViewIT.testMultipleValueDatarelied on both — it asserted"Johndoe"against a"JohnDoe"item and"name"against"namesurname", i.e. exactly the loophole being closed — and is updated to the real labels. It was the only test in the suite affected.Test coverage
New
AmbiguousLabelView+AmbiguousLabelIT: 12 tests, one per container, each holding an item whose label is a prefix of an earlier item's label, plus a duplicate-label case asserting the strict-mode error, plus a side-nav case checking a parent is found by its own label and is not returned for a child's../mvnw -Pit verify→ 717 ITs + 8 unit tests, all green.Docs (issue item 3)
AGENTS.md: "Exact vs substring matching" under the Scoped Lookup pattern, and a new pitfall Add vaadin-email-field #10 "Substring Label Matching".START-HERE.md(bundled in the jar): a fourth "easy to get wrong" entry, since this changes what offline agents should expect.docs/specifications/CheckboxGroupElement.md: it explicitly documented the old substring behaviour.api-reference.mdregenerated.Left out on purpose
SideNavigationItemElement.getLabel()/assertLabel()returns a parent item's whole subtree text ("Section\nSection 1\nSection 2"). Pre-existing, and an instance of the AGENTS.md "Light-DOM Text vstextContent" pitfall rather than a lookup-keying bug — worth its own issue..first()calls in shared part locators (HasLabelElement,HasPrefixElement, …): broader blast radius, unrelated to label keying.🤖 Generated with Claude Code