enkore / i3pystatus

A complete replacement for i3status
https://i3pystatus.readthedocs.io/
MIT License
445 stars 188 forks source link

HTML escape codes are not handled properly for pango markup. #794

Closed crocket closed 4 years ago

crocket commented 4 years ago

& is converted to &.

< is not converted to &lt;. &lt; is converted to &amp;lt;.

Refer to https://www.freeformatter.com/html-escape.html for details.

terminalmage commented 4 years ago

<, >, and " aren't converted because they are used as part of <span> tags which allow you to specifically colorize parts of the module's text.

crocket commented 4 years ago

I think i3pystatus shouldn't convert & to &amp; because I want to insert &lt; and so on.

terminalmage commented 4 years ago

That's fair, I think that the replace code can be altered to skip replacing that text when the & is part of an html escape.

terminalmage commented 4 years ago

@crocket https://github.com/enkore/i3pystatus/pull/795 should fix this, I just tested this on my install.

crocket commented 4 years ago

I think it's better and simpler to not expand &.

terminalmage commented 4 years ago

It is only expanded when the text is processed by pango, because if it is not the processing fails.

crocket commented 4 years ago

I think it's better for users to handle HTML escape codes... because expanding only & is confusing, and this behavior is not even documented.

terminalmage commented 4 years ago

expanding only & is confusing

Maybe that's the case for you, but I don't think you're considering the fact that not everyone understands HTML escapes. Many people will simply enable pango for a module because it enables specific colorization effects. For these users, adding a & to the format string will make the entire module's text disappear due to a failure of pango to process the text.

The pull request that I opened:

  1. Expands & to prevent confusion for those who don't have a more thorough understanding of pango / HTML escapes
  2. Fixes the bug in which & is expanded when part of an HTML escape code
  3. Does not prevent one from using &amp;, &lt;, &gt;, or &quot;

I'm not convinced that it's a great idea to make i3pystatus less intuitive for less-knowledgeable users, but it would be worth it IMO if there were good reasons for & not being expanded (such as it being needed as part of an HTML tag, like <, >, and "). I'm not aware of any such uses, but HTML and CSS are not my strong points. Are ampersands perhaps valid within, for example, a style attribute?

@enkore thoughts?

enkore commented 4 years ago

We're not using something like markupsafe, so we can't easily distinguish what came from the format string and what is e.g. the title of a song that's now playing (and happens to contain a &). I think escaping & when it is not part of an HTML entity is reasonable in this situation.

terminalmage commented 4 years ago

Oh yeah, I was so focused on the format string itself that I hadn't considered the module output (which is harder to control for).

crocket commented 4 years ago

Whatever. If it works, I'm okay.