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

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

Closed
HughePaul opened this issue Jun 20, 2021 · 7 comments · Fixed by #1893
Closed

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

HughePaul opened this issue Jun 20, 2021 · 7 comments · Fixed by #1893
Labels
bug Something isn't working
Milestone

Comments

@HughePaul
Copy link

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.

@HughePaul HughePaul added the bug Something isn't working label Jun 20, 2021
@ann0see
Copy link
Member

ann0see commented Jun 21, 2021

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 ann0see added this to the Release 3.8.1 milestone Jun 22, 2021
@ann0see
Copy link
Member

ann0see commented Jun 22, 2021

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.

@ann0see ann0see self-assigned this Jun 22, 2021
@ann0see ann0see removed their assignment Jun 22, 2021
@HughePaul
Copy link
Author

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
Copy link
Member

ann0see commented Jun 22, 2021

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
Copy link
Member

ann0see commented Jun 24, 2021

Ok. So the quick and dirty solution would be replace() here:

void CAudioMixerBoard::UpdateTitle()

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

ann0see added a commit to ann0see/jamulus that referenced this issue Jun 26, 2021
If a server-name contained '&' this was interpreted as keyboard shortcut.
Replacing & by && allows us to show server-names correctly.
Fixes: jamulussoftware#1886
@ann0see
Copy link
Member

ann0see commented Jun 26, 2021

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)?

ann0see added a commit to ann0see/jamulus that referenced this issue Jun 26, 2021
If a server-name contained '&' this was interpreted as keyboard shortcut.
Replacing & by && allows us to show server-names correctly.
Fixes: jamulussoftware#1886
ann0see added a commit to ann0see/jamulus that referenced this issue Jun 26, 2021
If a server-name contained '&' this was interpreted as keyboard shortcut in the mixer board.
Replacing & by && allows us to show server-names correctly.
Fixes: jamulussoftware#1886
ann0see added a commit to ann0see/jamulus that referenced this issue Jun 26, 2021
If a server-name contained '&' this was interpreted as keyboard shortcut in the mixer board.
Replacing & by && allows us to show server-names correctly.
Fixes: jamulussoftware#1886
@softins
Copy link
Member

softins commented Jun 28, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants