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

Fix the problem with displaying ampersand character in the entry remo... #9417

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

sewe2000
Copy link

…val dialog

I changed the TextType in MessageBox class to plain text so as to display the & character correctly and prevent HTML injection.
Fixes #9356

Screenshots

image

Type of change

  • ✅ Bug fix (non-breaking change that fixes an issue)
  • ✅ Breaking change (causes existing functionality to change)

@droidmonkey
Copy link
Member

droidmonkey commented May 14, 2023

This isn't the correct fix because you still want to HTML escape the title otherwise HTML elements in the title will be rendered like < and >. You should keep the HTML escaping but convert &amp; back to just & using a simple QString::replace.

I actually think this is a bonefide Qt bug in that it is not rendering escaped HTML characters correctly.

@phoerious
Copy link
Member

Qt is doing some on-the-fly content detection AFAIK, which is ridiculous. Setting it to a fixed value is definitely part of the solution.

@droidmonkey droidmonkey marked this pull request as draft May 14, 2023 20:40
@sewe2000 sewe2000 closed this May 15, 2023
@sewe2000 sewe2000 reopened this May 15, 2023
@sewe2000
Copy link
Author

OK, I made some fixes according to @droidmonkey's suggestions. Now it escapes HTML and displays & correctly.

@sewe2000 sewe2000 marked this pull request as ready for review May 16, 2023 19:43
droidmonkey
droidmonkey previously approved these changes May 19, 2023
@phoerious
Copy link
Member

I think the solution is incorrect. You are partially reverting your HTML escaping, which makes no sense to me. Either you escape or you don't and if you do, it either displays correctly (i.e., the message box accepts HTML) or it doesn't (then you get double escaping).

Instead of trying to mess with the escaping, you should set the textFormat property of the message box to PlainText. https://doc.qt.io/qt-6/qmessagebox.html#textFormat-prop
I think the default is AutoText, which is why sometimes you get HTML and sometimes you don't.

@droidmonkey droidmonkey self-requested a review October 24, 2023 09:37
@droidmonkey droidmonkey marked this pull request as draft October 24, 2023 09:37
@phoerious phoerious added pr: bugfix Pull request that fixes a bug and removed bug labels Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: bugfix Pull request that fixes a bug user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Entry deletion confirmation shows &amp; for ampersand
3 participants