<html>
    <head>
      <base href="https://bugzilla.gnome.org/" />
    </head>
    <body><span class="vcard"><a href="page.cgi?id=describeuser.html&login=jadahl%40gmail.com" title="Jonas Ådahl <jadahl@gmail.com>"> <span class="fn">Jonas Ådahl</span></a>
</span> changed
              <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [Wayland] Focused window under application first window"
   href="https://bugzilla.gnome.org/show_bug.cgi?id=780820">bug 780820</a>
          <br>
             <table border="1" cellspacing="0" cellpadding="8">
          <tr>
            <th>What</th>
            <th>Removed</th>
            <th>Added</th>
          </tr>

         <tr>
           <td style="text-align:right;">Attachment #351962 status</td>
           <td>none
           </td>
           <td>reviewed
           </td>
         </tr></table>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [Wayland] Focused window under application first window"
   href="https://bugzilla.gnome.org/show_bug.cgi?id=780820#c21">Comment # 21</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [Wayland] Focused window under application first window"
   href="https://bugzilla.gnome.org/show_bug.cgi?id=780820">bug 780820</a>
              from <span class="vcard"><a href="page.cgi?id=describeuser.html&login=jadahl%40gmail.com" title="Jonas Ådahl <jadahl@gmail.com>"> <span class="fn">Jonas Ådahl</span></a>
</span></b>
        <pre>Review of <span class=""><a href="attachment.cgi?id=351962&action=diff" name="attach_351962" title="[PATCH v3] wayland: Defer stack placement without a buffer">attachment 351962</a> <a href="attachment.cgi?id=351962&action=edit" title="[PATCH v3] wayland: Defer stack placement without a buffer">[details]</a></span> <a href='review?bug=780820&attachment=351962'>[review]</a>:

The solution looks sane to me, but I have some comments on the implementation.

::: src/core/stack.c
@@ +113,3 @@
+                  window->desc);
+      return;
+    }

I don't think we should have a function called "meta_stack_add()" that might
not add anything. We should also try to avoid any per-winsys code anywhere
outside of the MetaWindow(X11|Wayland|Xwayland) files.

An idea:

- Add MetaWindowClass::is_stackable()

- MetaWindowWayland implements this as "has buffer", MetaWindowX11 as
!override_redirect

- Add a meta_stack_is_window_added() helper

- In both _new_shared() and meta_window_show() do "if
(!meta_stack_is_window_added (stack, window) && meta_window_is_stackable
(window) meta_stack_add (stack, window)" and in unmanage do if
(meta_stack_is_window_added (stack, window)) meta_stack_remove (stack, window);
(alternatively add a meta_stack_maybe_add() that hides parts of the logic)

- We could remove the meta_bug() thing below, which would be dead code.</pre>
        </div>
      </p>
      <hr>
      <span>You are receiving this mail because:</span>
      
      <ul>
          <li>You are on the CC list for the bug.</li>
      </ul>
    </body>
</html>