Fix foreign toplevel handle eligibility and app_id handling for Wayland manager/list by Copilot · Pull Request #4946 · canonical/mir

Copilot · 2026-05-11T11:51:00Z

Related: n/a

What's new?

Foreign toplevel handle creation was over-inclusive (non-app windows), under-inclusive (freestyle windows), and sometimes emitted meaningless metadata (blank app_id/title). This change aligns both foreign-toplevel implementations to expose only eligible app toplevels with stable app identity.

Shared handle policy (deduplicated logic)
- Added foreign_toplevel_handle_creation.h with:
  - should_create_foreign_toplevel_handle(surface, app_id) (single eligibility gate)
- Removed the earlier foreign_toplevel_app_id(...) helper (it was only test-facing and unnecessary).
- Both foreign_toplevel_manager_v1.cpp and foreign_toplevel_list_v1.cpp use this shared policy.
Correct eligibility semantics
- Eligibility is now derived from a single surface plus app_id:
  - live session required
  - non-empty app_id required
  - depth layer must be mir_depth_layer_application
  - window must be focusable (focus_mode != mir_focus_mode_disabled)
  - supported types: normal, utility, and freestyle
- This excludes windows outside the application layer and windows that cannot gain focus.
- Freestyle windows are now correctly included.
Correct app_id emission semantics and performance
- Handle metadata prefers surface->application_id() when present.
- resolve_app_id() is called only when application_id is empty (including application_id_set_to paths), avoiding unnecessary expensive resolution.
- On application_id_set_to, eligibility is re-evaluated so handles can be created/closed as app identity becomes known.
Re-evaluate on depth-layer changes
- Added depth_layer_set_to(...) handling in both foreign-toplevel observers so eligibility updates when a surface moves between layers.
Focused tests
- Updated test_foreign_toplevel_handle_creation.cpp to validate the new surface-derived eligibility policy:
  - application-layer normal window allowed
  - freestyle window allowed
  - utility window allowed in application layer
  - non-application-layer windows denied
  - non-focusable windows denied
  - missing session denied
  - empty app_id denied
- Registered in frontend_wayland unit test CMake list.

How to test

Configure/build unit tests.
Run focused tests:
- mir_unit_tests.bin --gtest_filter=ForeignToplevelHandleCreation*
Run related regression slice:
- mir_unit_tests.bin --gtest_filter=DesktopFileManager*

Checklist

Tests added and pass
Adequate documentation added
(optional) Added Screenshots or videos

Agent-Logs-Url: https://github.com/canonical/mir/sessions/c43861cd-8934-4436-b91c-e180b8e155c4 Co-authored-by: AlanGriffiths <9048879+AlanGriffiths@users.noreply.github.com>

jhenstridge · 2026-05-11T15:25:38Z

-    else
-    {
-        should_have_handle = false;
+        app_id = foreign_toplevel_app_id(surface->application_id(), desktop_file_manager->resolve_app_id(surface.get()));


Calling resolve_app_id is potentially expensive. We should only call it if surface->application_id() is empty.

@copilot please address this thread

Updated in commit a5eeedf: resolve_app_id() is now only called when the surface/event application_id is empty (in both foreign_toplevel_list_v1.cpp and foreign_toplevel_manager_v1.cpp).

Agent-Logs-Url: https://github.com/canonical/mir/sessions/0e9c5b86-3758-4f59-8e49-373941df5945 Co-authored-by: AlanGriffiths <9048879+AlanGriffiths@users.noreply.github.com>

Copilot · 2026-05-11T19:33:53Z

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

dl.google.com
- Triggering command: /usr/lib/apt/methods/https /usr/lib/apt/methods/https (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

AlanGriffiths

@copilot please address issues raised in this therad

AlanGriffiths · 2026-05-12T06:24:01Z

+inline auto foreign_toplevel_app_id(std::string const& application_id, std::string const& resolved_app_id) -> std::string
+{
+    if (!application_id.empty())
+        return application_id;
+
+    return resolved_app_id;
+}
+


This is only used in tests: it isn't needed.

Addressed in 90f623e: removed the unused foreign_toplevel_app_id() helper from foreign_toplevel_handle_creation.h.

AlanGriffiths · 2026-05-12T06:27:23Z

+inline auto should_create_foreign_toplevel_handle(
+    MirWindowType type,
+    bool has_session,
+    std::string const& app_id) -> bool


All the parameters need to be derived from the same surface: it would be better to pass the surface than repeat this at each call site.

Addressed in 90f623e: should_create_foreign_toplevel_handle() now takes scene::Surface const& and derives eligibility inputs from that single surface.

AlanGriffiths · 2026-05-12T06:30:23Z

+{
+    return has_session && type == mir_window_type_normal && !app_id.empty();
+}


This will incorrectly identify windows outside the application layer and windows that cannot get focus. It will not identify freestyle windows.

Addressed in 90f623e: eligibility now requires application layer and focusable windows, and includes freestyle windows; also re-evaluates on depth-layer updates. Added focused coverage in e3b2678.

Agent-Logs-Url: https://github.com/canonical/mir/sessions/69ffaf31-6913-4e32-a808-bb4b8877913a Co-authored-by: AlanGriffiths <9048879+AlanGriffiths@users.noreply.github.com>

Copilot · 2026-05-12T06:50:52Z

@copilot please address issues raised in this therad

Addressed in 90f623e (logic updates) and e3b2678 (additional test coverage).

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

dl.google.com
- Triggering command: /usr/lib/apt/methods/https /usr/lib/apt/methods/https (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Initial plan

61152e6

Copilot AI assigned Copilot and AlanGriffiths May 11, 2026

Copilot started work on behalf of AlanGriffiths May 11, 2026 11:51 View session

Copilot AI linked an issue May 11, 2026 that may be closed by this pull request

Wrong ForeignToplevelHandleV1 creation logic #4944

Open

Copilot AI and others added 2 commits May 11, 2026 12:11

Fix foreign toplevel handle eligibility and app-id selection

887f694

Agent-Logs-Url: https://github.com/canonical/mir/sessions/c43861cd-8934-4436-b91c-e180b8e155c4 Co-authored-by: AlanGriffiths <9048879+AlanGriffiths@users.noreply.github.com>

Align new helper header license text with project style

d8653fd

Agent-Logs-Url: https://github.com/canonical/mir/sessions/c43861cd-8934-4436-b91c-e180b8e155c4 Co-authored-by: AlanGriffiths <9048879+AlanGriffiths@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix wrong ForeignToplevelHandleV1 creation logic~~ Fix foreign toplevel handle eligibility and app_id source for Wayland manager/list May 11, 2026

Copilot finished work on behalf of AlanGriffiths May 11, 2026 12:15

Copilot AI requested a review from AlanGriffiths May 11, 2026 12:15

jhenstridge reviewed May 11, 2026

View reviewed changes

Copilot started work on behalf of AlanGriffiths May 11, 2026 19:18 View session

Avoid eager resolve_app_id calls for foreign toplevel app id

a5eeedf

Agent-Logs-Url: https://github.com/canonical/mir/sessions/0e9c5b86-3758-4f59-8e49-373941df5945 Co-authored-by: AlanGriffiths <9048879+AlanGriffiths@users.noreply.github.com>

Copilot AI changed the title ~~Fix foreign toplevel handle eligibility and app_id source for Wayland manager/list~~ Fix foreign toplevel handle eligibility and app_id handling for Wayland manager/list May 11, 2026

Copilot finished work on behalf of AlanGriffiths May 11, 2026 19:34

AlanGriffiths reviewed May 12, 2026

View reviewed changes

Copilot started work on behalf of AlanGriffiths May 12, 2026 06:31 View session

Copilot AI and others added 2 commits May 12, 2026 06:46

Refine foreign toplevel eligibility policy per review feedback

90f623e

Agent-Logs-Url: https://github.com/canonical/mir/sessions/69ffaf31-6913-4e32-a808-bb4b8877913a Co-authored-by: AlanGriffiths <9048879+AlanGriffiths@users.noreply.github.com>

Add utility window eligibility coverage for foreign toplevel helper

e3b2678

Agent-Logs-Url: https://github.com/canonical/mir/sessions/69ffaf31-6913-4e32-a808-bb4b8877913a Co-authored-by: AlanGriffiths <9048879+AlanGriffiths@users.noreply.github.com>

Copilot finished work on behalf of AlanGriffiths May 12, 2026 06:51

Copilot AI requested a review from AlanGriffiths May 12, 2026 06:51

Conversation

Copilot AI commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's new?

How to test

Checklist

Uh oh!

jhenstridge May 11, 2026

Choose a reason for hiding this comment

Uh oh!

AlanGriffiths May 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented May 11, 2026

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

AlanGriffiths left a comment

Choose a reason for hiding this comment

Uh oh!

AlanGriffiths May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 12, 2026

Choose a reason for hiding this comment

Uh oh!

AlanGriffiths May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 12, 2026

Choose a reason for hiding this comment

Uh oh!

AlanGriffiths May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented May 11, 2026 •

edited

Loading

Copilot AI commented May 12, 2026 •

edited

Loading