fix(mpd): truncate before sanitization and improve tooltip UX#5064
Open
Siriusuna wants to merge 3 commits into
Open
fix(mpd): truncate before sanitization and improve tooltip UX#5064Siriusuna wants to merge 3 commits into
Siriusuna wants to merge 3 commits into
Conversation
Move the UTF-8 visual-width measurement and truncation helper functions from the mpris module to a common utility (include/util/utf8_string.hpp and src/util/utf8_string.cpp). This decouples string truncation and width measurement from the mpris module, allowing other modules (like mpd) to reuse. The helper function `utf8_truncate` (formerly `truncate` in mpris) and `utf8_width` is exported under the `waybar::util` namespace, while the low-level `measure_and_truncate` (formerly `utf8_truncate`) is encapsulated in an anonymous namespace in src/util/utf8_string.cpp to avoid unnecessary API exposure. No functional changes were made to the mpris module's behavior.
Fix an issue where the mpris module's tooltip failed to render when track metadata contained unescaped XML/HTML markup characters. This occurred because the mpris module formatted raw strings into the tooltip template before passing the final string directly to `set_tooltip_markup()`, triggering GTK/Pango parsing warnings and rendering failures. Resolve this by wrapping tooltip metadata fields in `Glib::Markup::escape_text` before formatting, ensuring Pango-compliant strings are always delivered to the GTK tooltip markup.
Fix an issue in the MPD module where text failed to render when raw tags containing markup characters were truncated mid-sequence after sanitization, resulting in fragmented XML entities (such as `&` cut into `&am`). Resolve this by shifting the order of operations to truncate the raw strings *before* running `sanitize_string`. To keep `setLabel` decoupled and clean, we introduce helper methods (`getArtistStr`, `getAlbumArtistStr`, `getAlbumStr`, `getTitleStr`) inside the `MPD` class, matching the architectural style of MPRIS. Additionally: - Introduce support for the customizable `ellipsis` config option (defaulting to `…`). - Switch the truncation logic of the MPD module to be visual-width-aware using the newly extracted `waybar::util::utf8_truncate`. - Decouple the tooltip text from the main label's truncated text. The tooltip now displays un-truncated, complete metadata (properly escaped), providing a significantly better user experience.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR centralizes UTF‑8 display-width measurement and truncation logic into a shared utility and updates media modules to use it, while also hardening MPRIS tooltip text handling.
Changes:
- Added
waybar::util::utf8_width/waybar::util::utf8_truncatehelpers and wired them into the build. - Replaced duplicated UTF‑8 truncate/width logic in MPRIS with the shared utility.
- Refactored MPD label/tooltip string construction to use helper getters and UTF‑8-aware truncation; added tooltip escaping for some MPRIS fields.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/util/utf8_string.cpp |
Implements shared UTF‑8 width + truncation logic. |
include/util/utf8_string.hpp |
Declares the new UTF‑8 string utility API. |
meson.build |
Adds the new utility source file to the build. |
src/modules/mpris/mpris.cpp |
Removes local UTF‑8 helpers; uses shared utility; escapes tooltip text for select fields. |
src/modules/mpd/mpd.cpp |
Uses shared UTF‑8 truncation and new getters for artist/album/title strings. |
include/modules/mpd/mpd.hpp |
Adds ellipsis_ and new string getter declarations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Author
|
It seems like my copilot auto review is somehow enabled. Sorry about the noises.😅 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
This PR addresses two related issues involving visual string truncation and XML/HTML markup escaping in the
mpdandmprismodules, preventing GTK/Pango markup parsing failures.sanitize_stringon raw tags before performing truncation. This introduces a bug where XML entities (such as&) are truncated mid-sequence (e.g., into&am). When passed to the Pango parser viaset_markup(), this triggers parsing errors, rendering the labels blank.&or<, passing the raw text toset_tooltip_markup()triggered GTK/Pango parsing warnings (e.g., "Entity did not end with a semicolon") and failed to render the tooltip entirely.Changes
utf8_truncate,utf8_width) frommpris.cppinto a common string utility underwaybar::util(include/util/utf8_string.hppandsrc/util/utf8_string.cpp) to enable clean code reuse.artist,title, andalbum) used in the MPRIS tooltip inGlib::Markup::escape_textbefore formatting, ensuring Pango-compliant strings are always delivered to the GTK tooltip markup.sanitize_string. This guarantees that truncation never cuts inside an escaped HTML/XML entity.getArtistStr,getAlbumArtistStr,getAlbumStr,getTitleStr) inside theMPDclass to keepsetLabel()clean and modular, matching the architectural style of the MPRIS module.ellipsisconfiguration option in the MPD module (consistent with MPRIS).Verification
"title-len","ellipsis"or keep them unset) and render contents containing markup characters verifying that: