fix: locator finds components placed in component slots

[mstahv]

Quickly vibed "solution" for another component tree traversal issues (case Card in my app). Haven't checked throuhgly, but I think we should (probably in some upcoming release try to resolve/unify all existing hacks and try to settle as much as possible for the new ComponentUtil.getAllChildren() method.

Claude's explanation why it was done this way:

Components like Card, Dialog, and similar slot-using parents add children via SlotUtils.addToSlot, which does plain parent.element.appendChild(child) + child.setAttribute("slot", ...). The slotted children are regular DOM children, just with a slot attribute. Card (and friends) then override getChildren() to filter out children with that attribute — so card.getChildren() only reports the content slot, hiding the rest.

TestingLifecycleHook.getAllChildren's default branch was

component.children.toList() + component._getVirtualChildren()

which trusts getChildren() and missed slotted children entirely. The locator API ended up unable to find buttons placed in a Card's header / header-prefix / header-suffix / footer slots even though they render in the browser.

Fix: in the same default branch, additionally pick up children with the slot attribute directly from the element tree. Conservative delta — keeps component.children as the primary source (so cases like ContextMenu, whose getChildren() returns logical items not represented in the element tree, still work), and adds slotted ones only when the component's own getChildren() filtered them out.

Repro test: CardSlotTraversalTest exercises all four Card slot setters plus add(). Without the fix, the first slotted assertion fails. Reduced from the standalone repro in tmp/card-locator-repro.

[mstahv]

This or some Card specific hack might be the right fix "for now", but we indeed need to get these issues resolved in some more structured way. Some analysis by Claude about the larger situation, these are NOT my recommendations:

Removing the component-tree-traversal workarounds: analysis & roadmap

The current implementation in TestingLifecycleHook.getAllChildren
(shared/src/main/kotlin/com/vaadin/browserless/internal/TestingLifecycleHook.kt)
is a when-block of special cases. Six branches, each papering over a
different mismatch between what Component.getChildren() reports and what a
tester actually wants to find. The recent Card-slot fix added a seventh patch
(see commit fix: locator finds components placed in component slots).

Inventory of the current workarounds

#	Case	What getChildren() returns	What we want	Workaround
1	Grid (commented out)	nothing useful — header/footer/editor live elsewhere	header cell components + footer cell components + editor components + body children	gather them from headerRows, footerRows, columns.editorComponent
2	MenuItemBase	only Element-level children	children + subMenu.items	manual concat
3	MenuBar	children (incl. items via MenuManager)	just children — virtuals would double-count	drop virtual children
4	Template (PolymerTemplate / LitTemplate)	children + shallow virtual-children shells	just children — virtual shells have no server-side state	drop virtual children
5	Grid.Column / ColumnGroup	children + virtual children = header/footer cell components	just children (the cell components belong to Grid, not to the column)	drop virtual children
6	Composite	the wrapped component, but getElement() is the wrapped element → virtuals get double-attributed	just children	drop virtual children
7	default else branch	children + virtual children — but misses slot-attribute regular DOM children that Card.getChildren() filters out	children + slotted-but-filtered children + virtual children	pick up slot-attribute children from the element tree (the new fix)

Where the asymmetry lives

Each workaround exists because the framework has no single, authoritative
answer to "what are this component's child components?" Components freely
use any combination of:

• Element.appendChild(...) with no further marking → counts as a regular
  Element child, reflected in getChildren() by default.
• Element.appendChild(...) + setAttribute("slot", ...) → counts as a
  regular Element child. Some components (Card) filter these out of their
  getChildren() override; others don't.
• Element.appendVirtualChild(...) (via VirtualChildrenList) → not in
  getChildren() by default; surfaces via ComponentUtil.getAllChildren() or
  Element.getVirtualChildren().
• A separate logical structure (MenuManager, header/footer row cells, editor
  components) → only reachable through component-specific API.
• Template-time element binding → creates "shell" components that exist
  server-side but aren't real children.

Different components mix-and-match these without any unifying contract. The
testing library is forced to know each component's specific bag of tricks.

What "fixing this properly" looks like

The principled fix is to have the component declare its own user-perceivable
child tree through a single, contractually-defined API. Three options:

Option 1: Fix Component.getChildren() itself (most idiomatic)

Make getChildren() return the user-perceivable tree everywhere. Most
components are already correct. The fixes needed:

Component	Change
Card	Stop filtering out slot-attribute children in getChildren(). They are children — they render in the browser. The current filter likely exists to keep removeAll() from removing slotted content, but those concerns can be separated (removeAll() can filter independently).
MenuItemBase	Override getChildren() to include subMenu.items (the items are logical children of the item server-side).
Grid	Override getChildren() to include header cell components + footer cell components + editor components + content children.
Grid.Column / ColumnGroup	Override getChildren() to exclude the header/footer cell components — those are owned by the Grid. (Likely they're in virtual children today, which super.getChildren() doesn't include — so this might already be the case; verify and document.)
Polymer/Lit template shells	Long-term: deprecate. Server-side template shell components are a legacy pattern in modern Vaadin. In the meantime, mark them with a marker interface (@TemplateRootBoundary or similar) so the framework can identify and skip them — see Option 3 below.

With getChildren() made correct, the entire when-block collapses to
component.children.toList(). The testing library becomes trivial.

Cost: 4–5 component-level PRs in Vaadin Flow / Flow components. Each is
small and the test surface is well-defined. Behavioral risk: medium-low —
getChildren() is consulted by app code, but the changes only ADD or
rearrange children it returns, never remove. Existing app code that iterates
getChildren() would just see more, which is mostly what users want.

Option 2: Promote ComponentUtil.getAllChildren() to instance API + override

Flow PR #24408 already added ComponentUtil.getAllChildren(component) as a
static helper. The next step:

public class Component {
    /**
     * Returns all children of this component that should be reachable for
     * testing, accessibility, and debug-tree purposes. Includes:
     *   - regular DOM children whose Element is a Component
     *   - children placed in slots via SlotUtils.addToSlot
     *   - virtual children attached via Element.appendVirtualChild
     *   - logical children stored outside the element tree (override to add)
     *
     * Default implementation walks the element tree (regular + virtual).
     * Components with logical children that aren't represented in the
     * element tree should override and include them.
     */
    public Stream<Component> getAllChildren() { ... }
}

MenuBar, ContextMenu, MenuItemBase, Grid etc. override; everyone else
inherits the default. The testing library calls component.getAllChildren()
and is done.

The split between getChildren() and getAllChildren() accommodates the
existing semantic load on getChildren() (used by removeAll(), layout DSLs,
etc.) where Card's filtering makes sense.

Cost: one core Flow PR + per-component overrides. Slightly more API
surface than Option 1, but doesn't risk changing existing getChildren()
semantics that app code may depend on.

Option 3: Marker interface(s) for capabilities

For the workarounds that exist because a category of component behaves
oddly (template shells, Composite element-aliasing), tag them with a marker:

• @TemplateRoot annotation or TemplateBoundary interface — testing
  library sees this and stops descending (replaces case chore: Make spotless work #4).
• Composite.getElement() aliasing — fix Composite itself to not return
  the wrapped element, OR add Composite to a "skip-virtual-children" marker
  interface (replaces case chore: Suppress irrelevant Flow warnings in browserless mode #6 — though Option 2's default behavior probably
  handles this already, since ComponentUtil.getAllChildren(composite)
  returns composite.getChildren() per the bytecode we inspected).

Recommended path: combine Options 1 + 2

A pragmatic, low-risk progression:

Phase A — component-side fixes (one Flow PR per component)

1. Card: stop filtering slot-attribute children in getChildren().
   Smallest, most isolated change. Removes the workaround we just merged.
2. MenuItemBase: include subMenu.items in getChildren(). Removes
   case ci: Add GitHub Actions validation workflow #2.
3. Grid: include header/footer/editor cell components in
   getChildren(). Removes case fix: Make ClassGraph discover package-private @Route views #1 (which is currently commented out, so
   this also unblocks Grid testing properly).
4. Audit other slot-using components (Dialog, MasterDetailLayout,
   Details, etc.) for the same Card-style filter; remove where present.

Each of these PRs is independent and lands at its own pace.

Phase B — Flow-side getAllChildren() instance method

Add Component.getAllChildren() as the canonical "testing tree" API.
Default implementation = ComponentUtil.getAllChildren(this).collect(Collectors.toList()).
Override on the few components that have logical-non-element children.

Phase C — browserless-test simplification

Once A and B are partially landed, the TestingLifecycleHook.getAllChildren
workarounds can be dismantled in a specific order:

fun getAllChildren(component: Component): List<Component> = when {
    // While Phase A migration is in flight, keep targeted workarounds
    // for un-migrated components — but each branch should reference a
    // Flow issue or commit that, once landed, makes the branch redundant.
    component.isTemplate -> ...               // until template shells go away
    // ... shrinking set ...
    else -> component.getAllChildren().toList()  // Phase B's instance method
}

Each Flow PR that lands lets us delete one branch.

Phase D — long-term

When the special cases are gone, TestingLifecycleHook.getAllChildren
collapses to:

fun getAllChildren(component: Component): List<Component> =
    component.getAllChildren().toList()

And the hook itself becomes vestigial — the only behavior left is
awaitBeforeLookup. At that point we can consider whether
TestingLifecycleHook should stay as an extensibility surface for tests (one
custom component that needs a tweak) or fold its awaitBeforeLookup into
MockVaadin and retire the hook entirely.

What to write up as Vaadin Flow issues

Concrete tickets a Flow PR could be raised against:

1. Card.getChildren() filters out slotted children — describe with the
   Card repro test from this PR. Propose unfiltering.
2. MenuItemBase.getChildren() omits sub-menu items — link to
   karibu-testing#76 (already referenced in our hook).
3. Grid.getChildren() omits header/footer/editor cell components —
   link to karibu-testing#52.
4. Proposal: Component.getAllChildren() instance method — feature
   request, builds on PR #24408.

What stays in browserless-test indefinitely

Realistically a few things will always need testing-library awareness:

• awaitBeforeLookup — the "client roundtrip before lookup" semantics
  is testing-specific. Not a tree-shape issue.
• Custom user components — a user with a weird custom container may need
  to register a TestingLifecycleHook of their own. The current extensibility
  mechanism is enough; we should keep it.
• Grid-row data fetching — _get(rowIndex) etc. needs to know about
  DataCommunicator. Not a tree-shape issue.

Effort estimate

• Phase A (4 Flow component PRs): each is 1–2 days of focused work + Vaadin
  Flow review cycle.
• Phase B (Component.getAllChildren() instance method): 2–3 days + a Flow
  API review.
• Phase C (browserless-test simplification): 1 day, gated on whatever Phase
  A/B lands first. Tracked as a per-branch deletion task.
• Total Vaadin-side: ~3 person-weeks across multiple Flow PRs.
• Total browserless-test-side after Vaadin lands: ~2–3 days of cleanup +
  test review.

The single highest-leverage change is fixing Card.getChildren() —
it's the bug that prompted this whole discussion, and it removes the
most-recently-added workaround. Followed closely by adding
Component.getAllChildren() as a real instance method — that's the API
contract piece that lets the testing library stop second-guessing each
component.