jamulussoftware / jamulus

Jamulus enables musicians to perform real-time jam sessions over the internet.
https://jamulus.io
Other
997 stars 222 forks source link

Ampersand isn't rendered in server name on mixer board title #1886

Closed HughePaul closed 3 years ago

HughePaul commented 3 years ago

Describe the bug Server names with ampersands (&) in them don't display the ampersand on the mixer board title

To Reproduce Connect to a server with a single ampersand somewhere in its name, eg: "Server & name" The mixerboard title shows as "Personal Mix at: Server name"

Expected behavior The ampersand should show up in the "Personal Mix at: Server & name" text

Operating system MacOSX 10.14.6, but likely to be all operating systems as this is a QT "feature".

Version of Jamulus 3.8.0

Screenshots

Screenshot 2021-06-20 at 21 33 53 Screenshot 2021-06-20 at 21 33 18

Additional context The mixerboard title is a QT label so it interprets ampersand as a keyboard shortcut binding and the ampersand won't be shown, but the following letter may have an underline, depending on the operating system (it doesn't on mac, but does on linux)

To escape the ampersand chars they should be doubled up before being set to the label (eg: "Server && name") Something like

strServerName.replace("&", "&&");

should suffice in the correct place, as long as that doesn't get run on the same string instance multiple times, and mustn't pollute the server list string, or the window title string, which both display correctly.

ann0see commented 3 years ago

Thanks for the bug report and a solution. I assume if the & gets interpreted, there might also be other special characters. The client should replace them like PHP‘s htmlspecialchars(); Feel free to open a pull request with your fix!

ann0see commented 3 years ago

Ok. Can reproduce the bug on Debian Linux.

strServerName.replace("&", "&&");

I don't think this actually solves the problem:

Let's think of a server called "(a && b)". This would then result in the label saying "(a & b)" although the server is not called like that. So we need something like htmlspecialchars() here.

Although they're talking about python here: https://stackoverflow.com/questions/47584343/how-to-set-text-in-a-qlabel-and-display-characters

label.setTextFormat(QtCore.Qt.PlainText)

maybe something like that could also be applied to C++: https://doc.qt.io/qt-5/qlabel.html#textFormat-prop

However, this might break some server names? Not sure...

Or maybe it's just as easy as adding .toHtmlEscaped()...: https://doc.qt.io/qt-5/qstring.html#toHtmlEscaped (doesn't seem so...)

Ok. we should look into audiomixerboard.cpp, find the label and read the code around the cration of the label.

HughePaul commented 3 years ago

The string "(a && b)" should be set to a label as "(a &&&& b)" to show correctly, which the replace should do.

It should only be the ampersand that gets treated specially by the qt labels. If you html escape them you'll see something like "(a amp;amp; b)"

ann0see commented 3 years ago

Yes, I didn’t test replace() yet.

It just doesn’t feel right to fix this by replacing & with &&.

Yes, .toHtmlEscaped(); doesn’t fix it.

ann0see commented 3 years ago

Ok. So the quick and dirty solution would be replace() here: https://github.com/jamulussoftware/jamulus/blob/1796a98f82fc23d2f3fd8b25fecc738aca4f48d7/src/audiomixerboard.cpp#L1096

I‘d be happy if we found a better solution though.

ann0see commented 3 years ago

Just raised #1893. Your solution seems to be the only way I found to solve this issue.

Could you please test if the change does what you want (especially on macOS and Windows if possible)?

softins commented 3 years ago

Doubling the & is in fact the correct solution. See https://doc.qt.io/qt-5/qgroupbox.html#title-prop