fix(tray): Allow module to disappear for empty tray

Modules that don't produce any output are hidden by the controller
(don't have margins or separators).
The tray module should also do that for `format = <tray>` when there are
no icons.

This required the visibility handling to be tied to the module
visibility instead of being handled by the renderer.
Otherwise, the renderer would hide the tray (because the %{Pt} tag was
never sent) and the tray would not unhide when new icons appeared; it
can't differentiate between hidden because empty and hidden because the
module is hidden by the user (the latter is the reason the renderer does
hiding at all).

Fixes #3036

CHANGELOG.md

@@ -9,6 +9,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Changed
+- `internal/tray`: The module must use the `<tray>` tag (this is the default) ([`#3037`](https://github.com/polybar/polybar/pull/3037))
+
+## Fixed
+- `internal/tray`: Fixed `module-margin` and `separator` being applied to completely empty tray module ([`#3036`](https://github.com/polybar/polybar/issues/3036), [`#3037`](https://github.com/polybar/polybar/pull/3037))
 
 ## [3.7.0] - 2023-11-05
 ### Breaking

include/modules/meta/base.hpp

@@ -189,7 +189,7 @@ namespace modules {
     string get_format() const;
     string get_output();
 
-    void set_visible(bool value);
+    virtual void set_visible(bool value);
 
     void action_module_toggle();
     void action_module_show();

include/modules/tray.hpp

@@ -8,11 +8,21 @@
 
 POLYBAR_NS
 namespace modules {
+/**
+ * Wraps the tray_manager in a module.
+ *
+ * The module produces the `%{Pt}` formatting tag, which is used by the renderer
+ * to place the tray.
+ * The visibility of the tray icons is directly tied to the visibility of the
+ * module.
+ */
 class tray_module : public static_module<tray_module> {
  public:
   explicit tray_module(const bar_settings& bar_settings, string name_, const config&);
   string get_format() const;
 
+  void set_visible(bool value) override;
+
   void start() override;
 
   bool build(builder* builder, const string& tag) const;

include/x11/tray_manager.hpp

@@ -103,6 +103,8 @@ class manager : public xpp::event::sink<evt::expose, evt::client_message, evt::c
 
   bool is_visible() const;
 
+  bool change_visibility(bool visible);
+
  protected:
   void recalculate_width();
   void reconfigure_clients();
@@ -130,7 +132,6 @@ class manager : public xpp::event::sink<evt::expose, evt::client_message, evt::c
   void remove_client(const client& client);
   void remove_client(xcb_window_t win);
   void clean_clients();
-  bool change_visibility(bool visible);
 
   void handle(const evt::expose& evt) override;
   void handle(const evt::client_message& evt) override;

src/components/renderer.cpp

@@ -839,13 +839,10 @@ bool renderer::on(const signals::ui::request_snapshot& evt) {
 }
 
 void renderer::apply_tray_position(const tags::context& context) {
-  if (context.get_relative_tray_position() != std::pair<alignment, int>()) {
-    int absolute_x = static_cast<int>(
-        block_x(context.get_relative_tray_position().first) + context.get_relative_tray_position().second);
+  auto [alignment, pos] = context.get_relative_tray_position();
+  if (alignment != alignment::NONE) {
+    int absolute_x = static_cast<int>(block_x(alignment) + pos);
     m_sig.emit(signals::ui_tray::tray_pos_change{absolute_x});
-    m_sig.emit(signals::ui_tray::tray_visibility{true});
-  } else {
-    m_sig.emit(signals::ui_tray::tray_visibility{false});
   }
 }
 

src/modules/tray.cpp

@@ -8,27 +8,46 @@ namespace modules {
   template class module<tray_module>;
 
   tray_module::tray_module(const bar_settings& bar_settings, string name_, const config& config)
-      : static_module<tray_module>(bar_settings, move(name_), config)
+      : static_module<tray_module>(bar_settings, std::move(name_), config)
       , m_tray(connection::make(), signal_emitter::make(), m_log, bar_settings, [&] { this->broadcast(); }) {
     m_formatter->add(DEFAULT_FORMAT, TAG_TRAY, {TAG_TRAY});
+
+    /* There are a bunch of edge cases with regards to tray visiblity when the
+     * <tray> tag is not there (in that case the tray icons should under no
+     * circumnstances appear). To avoid this, we simply disallow the situation.
+     * The module is basically useless without that tag anyway.
+     */
+    if (!m_formatter->has(TAG_TRAY, DEFAULT_FORMAT)) {
+      throw module_error("The " + std::string(TAG_TRAY) + " tag is required in " + name() + "." + DEFAULT_FORMAT);
+    }
+
+    // Otherwise the tray does not see the initial module visibility
+    m_tray.change_visibility(visible());
   }
 
   string tray_module::get_format() const {
     return DEFAULT_FORMAT;
   }
 
+  void tray_module::set_visible(bool value) {
+    m_tray.change_visibility(value);
+    static_module<tray_module>::set_visible(value);
+  }
+
   void tray_module::start() {
     m_tray.setup(m_conf, name());
     this->static_module<tray_module>::start();
   }
 
   bool tray_module::build(builder* builder, const string& tag) const {
-    if (tag == TAG_TRAY) {
+    // Don't produce any output if the tray is empty so that the module can be hidden
+    if (tag == TAG_TRAY && m_tray.get_width() > 0) {
       builder->control(tags::controltag::t);
       extent_val offset_extent = {extent_type::PIXEL, static_cast<float>(m_tray.get_width())};
       builder->offset(offset_extent);
       return true;
     }
+
     return false;
   }
 

src/x11/tray_manager.cpp

@@ -522,7 +522,7 @@ void manager::clean_clients() {
 }
 
 bool manager::change_visibility(bool visible) {
-  if (!is_active() || m_hidden == !visible) {
+  if (m_hidden == !visible) {
     return false;
   }