Skip to content

fix(PlanView): reload mission item editors from tree root to avoid stale-delegate loads#14668

Closed
DonLakeFlyer wants to merge 1 commit into
mavlink:masterfrom
DonLakeFlyer:fix-plantree-stale-editor-reload
Closed

fix(PlanView): reload mission item editors from tree root to avoid stale-delegate loads#14668
DonLakeFlyer wants to merge 1 commit into
mavlink:masterfrom
DonLakeFlyer:fix-plantree-stale-editor-reload

Conversation

@DonLakeFlyer

Copy link
Copy Markdown
Contributor

Problem

PlanViewLayerUITest fails intermittently in CI (most recently on #14667) with strict-mode log failures:

QQmlContext: Cannot set context object on invalid context.
QQmlComponent: Cannot create a component in an invalid context

This is the CI-only "invalid context" flavor noted as unresolved in #14638 ("never reproduced locally in ~340 runs... if it recurs in CI after this change it can be hunted separately"). It recurred.

Root cause

MissionItemEditor.qml used a Connections { target: missionItem } to reload its editor Loader on isCurrentItemChanged. TreeView (with reuseItems: false) releases delegates by invalidating their QML context immediately while deferring actual destruction via deleteLater. In that window the Connections to the persistent MissionItem C++ object still fires, and Loader.setSource() attempts a component load in the invalidated context. QQuickLoaderPrivate::_q_sourceLoaded() then does new QQmlContext(creationContext) + setContextObject() on the dead context — producing exactly the two warnings above.

The race needs delegate teardown (layer switch / group collapse / forceLayout) to coincide with a current-item change — e.g. the takeoff-item insertion in test step 2.2 — and is far more likely on slow CI runners, which is why it never reproduced locally.

Fix

Invert the control:

  • Remove the per-delegate Connections from MissionItemEditor.qml; rename _loadEditor() to public reloadEditor().
  • PlanTreeView.qml (whose root outlives all delegates) reacts to MissionController::planViewStateChanged — emitted whenever the current plan-view item changes — and calls reloadEditor() only on delegates that are still live, via itemAtCell() over the visible rows. Released delegates are unreachable through itemAtCell, so no load can ever be triggered in an invalidated context.
  • Newly created delegates still load their editor from Component.onCompleted.

Testing

  • 100 consecutive local runs of PlanViewLayerUITest with the fix (30 direct binary + 70 via ctest): 0 failures.
  • PlanViewUITest and ToolbarIndicatorUITest pass (editor expand/collapse regression check).
  • Note: the invalid-context flavor only ever reproduced in CI, so CI is the real verifier here.

Copilot AI review requested due to automatic review settings July 21, 2026 03:38

This comment was marked as resolved.

Copilot AI review requested due to automatic review settings July 21, 2026 03:43
@DonLakeFlyer
DonLakeFlyer force-pushed the fix-plantree-stale-editor-reload branch from 478285e to 3aa8786 Compare July 21, 2026 03:43

This comment was marked as resolved.

…ale-delegate loads

MissionItemEditor used a Connections on its missionItem to reload the
editor Loader whenever isCurrentItemChanged fired. TreeView releases
delegates with reuseItems: false by invalidating their QML context
immediately while deferring object destruction via deleteLater. In that
window the Connections to the persistent MissionItem still fires, and
the Loader's setSource() then attempts a component load in the
invalidated context, producing:

  QQmlContext: Cannot set context object on invalid context.
  QQmlComponent: Cannot create a component in an invalid context

which fails PlanViewLayerUITest under strict-mode log checking. The
race only triggers when delegate teardown (layer switch, group
collapse, forceLayout) coincides with a current-item change - e.g.
takeoff item insertion - and is far more likely on slow CI runners.

Invert the control: remove the per-delegate Connections and have
PlanTreeView (whose root outlives all delegates) react to
MissionController::planViewStateChanged and call reloadEditor() only on
delegates that are still live via itemAtCell(). Newly created delegates
load their editor from Component.onCompleted as before.
Copilot AI review requested due to automatic review settings July 21, 2026 03:54
@DonLakeFlyer
DonLakeFlyer force-pushed the fix-plantree-stale-editor-reload branch from 3aa8786 to 2c2df3a Compare July 21, 2026 03:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Build results unavailable — artifact download from one or more platform workflows failed (likely artifact retention expiry or transient API error). The combined report cannot be generated for this run.

See the Build Results workflow run for details.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants