This is an automated email from the git hooks/post-receive script.

git pushed a commit to branch wl/browser-all
in repository enlightenment.

View the commit online.

commit 5cbf366e0ba4088fc5e822c792f4bc752a6a012b
Author: Cedric BAIL <[email protected]>
AuthorDate: Sat Aug 8 21:58:38 2026 -0600

    e_comp_wl - allow attach-before-configure under test only
    
    wlcs cannot construct a surface against a spec-correct compositor.
    
    XdgStableSurfaceBuilder::build() creates the xdg_surface and xdg_toplevel
    and calls attach_visible_buffer() immediately - no initial empty commit, no
    waiting for xdg_surface.configure. xdg-shell says plainly that this is not
    allowed:
    
      "any attempts by a client to attach or manipulate a buffer prior to the
       first xdg_surface.configure call must also be treated as errors"
    
    So E is right and wlcs is wrong. Still unchanged as of wlcs 933bc61
    (post-1.8.2), which means every compositor currently passing those tests is
    being lenient about a documented protocol error. E is not being changed to
    match, and this is not a workaround for a misbehaving client.
    
    The problem is only that ~315 of 1163 tests die at surface construction and
    never reach what they were written to exercise. That hides real defects:
    with the relaxation, the first such test gets past attach and then times out
    waiting for an interactive move that never happens - a genuine gap that was
    invisible before.
    
    So: strict by default, relaxed only when a test asks, via
    E_TEST_ALLOW_UNCONFIGURED_BUFFER. Compiled in only under -Dtests=true
    (E_TESTS in config.h), so a release build cannot reach it whatever the
    environment says - verified by inspecting config.h in a -Dtests=false build.
    run-nested.sh deliberately does not set it: our own tests follow the
    protocol and should not be able to hide behind it.
    
    The comment at the site carries the spec quote and links to both the current
    xdg-shell.xml and the wlcs source, so the next person does not have to
    re-derive who is wrong. If wlcs grows the initial commit, delete all of it.
    
    Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
    Claude-Session: https://claude.ai/code/session_01FtoiXoSKUmZb6Aix6U3GZS
---
 meson.build                     |  7 ++++
 src/bin/e_comp_wl.c             | 71 +++++++++++++++++++++++++++++++++++++++--
 src/tests/wayland/run-nested.sh |  4 +++
 src/tests/wlcs/README.md        | 37 +++++++++++++++++++++
 src/tests/wlcs/e_wlcs.c         |  9 ++++++
 5 files changed, 126 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index d141b2865..a0bee8aff 100644
--- a/meson.build
+++ b/meson.build
@@ -428,6 +428,13 @@ if config_h.has('HAVE_WAYLAND') == true
 	wayland = 'true'
 endif
 
+if get_option('tests') == true
+  # Compiles in the test-only protocol relaxations. Never defined in a release
+  # build, so those code paths cannot be reached by an installed compositor
+  # even with the environment variables set.
+  config_h.set('E_TESTS', '1')
+endif
+
 configure_file(output       : 'config.h',
                configuration: config_h,
                install_dir  : dir_include_e)
diff --git a/src/bin/e_comp_wl.c b/src/bin/e_comp_wl.c
index 5fe7176c1..99e16295c 100644
--- a/src/bin/e_comp_wl.c
+++ b/src/bin/e_comp_wl.c
@@ -1601,6 +1601,66 @@ _e_comp_wl_surface_cb_destroy(struct wl_client *client EINA_UNUSED, struct wl_re
    wl_resource_destroy(resource);
 }
 
+#ifdef E_TESTS
+/* Tolerate a client attaching a buffer before its first xdg_surface.configure.
+ *
+ * This is a PROTOCOL VIOLATION and we only ever allow it under -Dtests=true,
+ * with E_TEST_ALLOW_UNCONFIGURED_BUFFER set. It is never compiled into a
+ * release build. Do not enable it to "fix" a misbehaving client.
+ *
+ * What the protocol says
+ * ----------------------
+ * xdg-shell, xdg_surface description - stable/xdg-shell/xdg-shell.xml, at
+ * https://gitlab.freedesktop.org/wayland/wayland-protocols/-/blob/main/stable/xdg-shell/xdg-shell.xml
+ *
+ *   "Creating an xdg_surface from a wl_surface which has a buffer attached or
+ *    committed is a client error, and any attempts by a client to attach or
+ *    manipulate a buffer prior to the first xdg_surface.configure call must
+ *    also be treated as errors."
+ *
+ *   "After creating a role-specific object and setting it up, the client must
+ *    perform an initial commit without any buffer attached. [...] The client
+ *    must acknowledge it and is then allowed to attach a buffer to map the
+ *    surface."
+ *
+ * So rejecting this is correct, and xdg_surface.error.unconfigured_buffer
+ * exists precisely to say so. We are not relaxing the rule because we think
+ * the rule is wrong.
+ *
+ * Why we relax it for tests
+ * -------------------------
+ * wlcs, the Wayland conformance suite, does not perform the initial commit.
+ * XdgStableSurfaceBuilder::build() in
+ * https://github.com/canonical/wlcs/blob/main/src/surface_builder.cpp
+ * creates the xdg_surface and xdg_toplevel and calls attach_visible_buffer()
+ * straight away; XdgV6SurfaceBuilder and the create_xdg_shell_*_surface
+ * helpers in src/in_process_server.cpp do the same. Verified unchanged as of
+ * wlcs 933bc61 (post-1.8.2), so this is not a stale-version problem, and it
+ * implies every compositor currently passing those tests is being lenient
+ * about a documented error.
+ *
+ * The practical consequence is that ~315 of 1163 wlcs tests die at surface
+ * construction against a spec-correct compositor, and whatever they were
+ * meant to exercise is never reached. That hides real defects: with this
+ * relaxation the first such test gets past attach and then times out waiting
+ * for an interactive move that does not happen - a genuine gap we could not
+ * see before.
+ *
+ * So: strict by default, lenient only when a test asks, and the difference
+ * between the two runs is itself a useful measurement.
+ *
+ * Revisit if wlcs grows the initial commit - then this can simply be deleted.
+ */
+static Eina_Bool
+_e_comp_wl_test_allow_unconfigured_buffer(void)
+{
+   static int allow = -1;
+
+   if (allow < 0) allow = !!getenv("E_TEST_ALLOW_UNCONFIGURED_BUFFER");
+   return allow;
+}
+#endif
+
 static void
 _e_comp_wl_surface_cb_attach(struct wl_client *client, struct wl_resource *resource, struct wl_resource *buffer_resource, int32_t sx, int32_t sy)
 {
@@ -1611,8 +1671,15 @@ _e_comp_wl_surface_cb_attach(struct wl_client *client, struct wl_resource *resou
    if (e_object_is_del(E_OBJECT(ec))) return;
    if (ec->comp_data->need_xdg_configure)
      {
-        ec->comp_data->shell.buffer_attach_error(ec);
-        return;
+#ifdef E_TESTS
+        if (_e_comp_wl_test_allow_unconfigured_buffer())
+          ec->comp_data->need_xdg_configure = 0;
+        else
+#endif
+          {
+             ec->comp_data->shell.buffer_attach_error(ec);
+             return;
+          }
      }
 
    if (buffer_resource)
diff --git a/src/tests/wayland/run-nested.sh b/src/tests/wayland/run-nested.sh
index 6c16b4605..41bef09a7 100755
--- a/src/tests/wayland/run-nested.sh
+++ b/src/tests/wayland/run-nested.sh
@@ -138,6 +138,10 @@ trap cleanup EXIT INT TERM
 # absolute .so path, which e_module_new() has always handled.
 E_TEST_MODULE=${E_TEST_MODULE:-wl_test}
 
+# Off here: our own tests follow xdg-shell properly, so they have no need of
+# the relaxation and should not be able to hide behind it. run-nested.sh is
+# for tests we wrote; the wlcs integration enables it for tests we did not.
+
 # The compositor's own environment. Deliberately not exported to this shell,
 # so the client below is launched with WAYLAND_DISPLAY pointing at the nested
 # compositor and nothing else inherited by accident.
diff --git a/src/tests/wlcs/README.md b/src/tests/wlcs/README.md
index 2540df238..e7ea1243f 100644
--- a/src/tests/wlcs/README.md
+++ b/src/tests/wlcs/README.md
@@ -64,3 +64,40 @@ it quietly makes them exercise an older protocol than E actually speaks, and
 they pass. So the list is checked rather than trusted — `e_wlcs_driver` walks
 the compositor's real registry and fails if the two disagree in either
 direction: a version E does not have, or an interface it does not advertise.
+
+## The attach-before-configure relaxation
+
+By default the integration sets `E_TEST_ALLOW_UNCONFIGURED_BUFFER=1` in the
+compositor it launches. Run with `E_WLCS_STRICT=1` to turn that off.
+
+wlcs builds every test surface without the initial empty commit xdg-shell
+requires — `XdgStableSurfaceBuilder::build()` in
+[`src/surface_builder.cpp`](https://github.com/canonical/wlcs/blob/main/src/surface_builder.cpp)
+creates the `xdg_surface` and `xdg_toplevel` and calls `attach_visible_buffer()`
+immediately. The spec is explicit that this is an error
+([xdg-shell.xml](https://gitlab.freedesktop.org/wayland/wayland-protocols/-/blob/main/stable/xdg-shell/xdg-shell.xml),
+`xdg_surface` description):
+
+> any attempts by a client to attach or manipulate a buffer prior to the first
+> `xdg_surface.configure` call must also be treated as errors
+
+So E is right to reject it and wlcs is wrong — still unchanged as of wlcs
+`933bc61`, which means every compositor passing these tests is tolerating a
+documented protocol error. **E is not being changed to match.**
+
+The problem is purely that ~315 tests then die at surface construction and
+never reach what they were written to check. Measured: with the relaxation,
+`XdgToplevelStableTest.surface_can_be_moved_interactively` gets past attach and
+times out on an interactive move that never happens — a real gap the strict
+path was hiding.
+
+The relaxation is compiled in only under `-Dtests=true` (`E_TESTS` in
+`config.h`), so a release build cannot reach that code path whatever the
+environment says — verified by checking `config.h` in a `-Dtests=false` build.
+It is deliberately **not** enabled for `run-nested.sh`: our own tests follow
+the protocol properly and should not be able to hide behind it.
+
+Running both ways is itself informative — the difference is exactly the set of
+tests wlcs's violation is responsible for.
+
+If wlcs ever grows the initial commit, delete the relaxation and this section.
diff --git a/src/tests/wlcs/e_wlcs.c b/src/tests/wlcs/e_wlcs.c
index 09ac2f2bd..e9779e6d1 100644
--- a/src/tests/wlcs/e_wlcs.c
+++ b/src/tests/wlcs/e_wlcs.c
@@ -252,6 +252,15 @@ _server_start(WlcsDisplayServer *server)
         setenv("E_CONF_PROFILE_NOSAVE", "1", 1);
         setenv("E_MODULE_FORCE_LOAD", "wl_test", 1);
 
+        /* wlcs builds every surface without the initial empty commit that
+         * xdg-shell requires, so a spec-correct compositor kills the client
+         * at surface construction and ~315 tests never reach what they are
+         * meant to test. Opt in to the relaxation unless asked not to - see
+         * the long note in e_comp_wl.c. Set E_WLCS_STRICT=1 to measure how
+         * many tests that violation is responsible for. */
+        if (!getenv("E_WLCS_STRICT"))
+          setenv("E_TEST_ALLOW_UNCONFIGURED_BUFFER", "1", 1);
+
         setenv("E_PREFIX", prefix, 1);
         snprintf(path, sizeof(path), "%s/bin", prefix);
         setenv("E_BIN_DIR", path, 1);

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.

Reply via email to