Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

panel: sanitize variables added to markup #268

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

trigg
Copy link
Contributor

@trigg trigg commented Jul 30, 2024

I've intentionally side-stepped touching command-output as it is fully possible that some users are intentionally writing markup out from a command to the panel.

@trigg
Copy link
Contributor Author

trigg commented Jul 30, 2024

Fixes #266

@NamorNiradnug
Copy link
Collaborator

Hi! Thanks for your PR.
Current behavior is actually not a bug but a feature because StatusNotifierItem protocol allows markup tags in tooltip text: https://www.freedesktop.org/wiki/Specifications/StatusNotifierItem/Markup/

@trigg
Copy link
Contributor Author

trigg commented Jul 31, 2024

Interesting, that is an issue.

This came to light because an app (Spotify i believe) was using a loose ampersand which was causing a warning and incorrect label.

@NamorNiradnug
Copy link
Collaborator

NamorNiradnug commented Jul 31, 2024

What if we apply escape_text on tooltip_title only, but not on tooltip_text?

@trigg
Copy link
Contributor Author

trigg commented Aug 5, 2024

Looking back at the initial bug, yes that would cover it. Might still be scope for the bug to creep back in as the documentation you linked doesn't mention escaping & at all.

@NamorNiradnug NamorNiradnug self-requested a review August 5, 2024 17:23
@NamorNiradnug
Copy link
Collaborator

Looking back at the initial bug, yes that would cover it. Might still be scope for the bug to creep back in as the documentation you linked doesn't mention escaping & at all.

I think that's a good solution then because it's simple and the specification doesn't mean markup for title. Escaping & and other symbols manually would make the code complicated and I don't think it is worth it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants