zwlr_foreign_toplevel_handle_v1.app_id() does not respect xdg_toplevel.set_app_id()

[AlanGriffiths]

Running the wlcs tests from within CLion I see:

WAYLAND_DEBUG=client cmake-build-debug/wlcs /home/alan/CLionProjects/mir/cmake-build-debug/lib/miral_wlcs_integration.so --gtest_filter=ForeignToplevelHandleTest.can_minimize_foreign  
...
[1105973.424]  -> [email protected]_app_id("fake.wlcs.app.id")
...
[1105974.183] [email protected]_id("clion_clion.desktop")

Running the same test from a console window:

WAYLAND_DEBUG=client cmake-build-debug/wlcs /home/alan/CLionProjects/mir/cmake-build-debug/lib/miral_wlcs_integration.so --gtest_filter=ForeignToplevelHandleTest.can_minimize_foreign  
...
[1267160.475]  -> [email protected]_app_id("fake.wlcs.app.id")
...
[1267160.901] [email protected]_id("fake.wlcs.app.id")

The latter is what I (and the wlcs code) expect.

I guess that the difference is that CLion is a classic snap (with an associated AppArmor profile). But if a client sets the app_id, what business do we have in overriding it? Especially with a .desktop file name?

[AlanGriffiths]

A workaround is to prefix the wlcs command with aa-exec -p unconfined vis:

aa-exec -p unconfined cmake-build-debug/wlcs /home/alan/CLionProjects/mir/cmake-build-debug/lib/miral_wlcs_integration.so --gtest_filter=ForeignToplevelHandleTest.can_minimize_foreign

[AlanGriffiths]

Especially with a .desktop file name?

That bit is likely #3608

[mattkae]

The real issue here is that we're abusing the zwlr_foreign_toplevel_handle_v1.app_id() field, and we've known this from the start. Instead of just returning the app ID as you gave it to us, we instead try to resolve it to a value that is more useful when resolving desktop files on the client's side (e.g. the app armor profile).

So, even if the user has set an app_id, we instead use that to resolve a desktop file. Maybe we should just be introducing a new field here, but that is a design decisions that we'd have to make.