Skip to content
This repository was archived by the owner on May 21, 2026. It is now read-only.

Remove html filter#19

Open
EstatoDeviato wants to merge 8 commits into
Killing-Fever-Online:masterfrom
EstatoDeviato:remove-html-filter
Open

Remove html filter#19
EstatoDeviato wants to merge 8 commits into
Killing-Fever-Online:masterfrom
EstatoDeviato:remove-html-filter

Conversation

@EstatoDeviato

@EstatoDeviato EstatoDeviato commented Jun 10, 2024

Copy link
Copy Markdown
Collaborator

Removing html filter, to allow users to use bold, italic, colors ecc... in ooc message

@in1tiate

in1tiate commented Jun 16, 2024

Copy link
Copy Markdown

You need to add parsing to catch open tags. With this change as-is, I can write:

<font face="Comic Sans MS">

and deliberately not include the closing </font> tag, which will cause all messages sent thereafter to be in Comic Sans until someone closes the tag. This works with any formatting tag and will cause you headaches if you don't address it.

Alternatively, you could choose the sane option and not remove the HTML filter, which is there for a reason.

@Crystalwarrior Crystalwarrior left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PLEASE descriptive descriptions I beg

Comment thread src/aotextarea.cpp Outdated
QString result = p_message.toHtmlEscaped()
.replace("\n", "<br>")
.replace(url_parser_regex, "<a href='\\1'>\\1</a>");
QString result = p_message.replace("\n", "<br>").replace(url_parser_regex, "<a href='\\1'>\\1</a>");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to close all open HTML tags to prevent the Comic Sans disaster

Comment thread src/aotextarea.cpp Outdated
Comment on lines +84 to +123
std::string AOTextArea::closetags(std::string html)
{
std::regex opened_regex(R"(<([a-z]+)(?: .*)?(?<![/|/ ])>)",
std::regex_constants::icase);
std::regex closed_regex(R"(</([a-z]+)>)", std::regex_constants::icase);

std::vector<std::string> openedtags;
std::vector<std::string> closedtags;
std::smatch match;

std::string::const_iterator search_start(html.cbegin());
while (std::regex_search(search_start, html.cend(), match, opened_regex)) {
openedtags.push_back(match[1]);
search_start = match.suffix().first;
}

search_start = html.cbegin();
while (std::regex_search(search_start, html.cend(), match, closed_regex)) {
closedtags.push_back(match[1]);
search_start = match.suffix().first;
}

size_t len_opened = openedtags.size();

if (closedtags.size() == len_opened) {
return html;
}

std::reverse(openedtags.begin(), openedtags.end());
for (size_t i = 0; i < len_opened; i++) {
auto it = std::find(closedtags.begin(), closedtags.end(), openedtags[i]);
if (it == closedtags.end()) {
html += "</" + openedtags[i] + ">";
}
else {
closedtags.erase(it);
}
}
return html;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you rewrite this to use the Qt functions rather than std:: ones? we must be breaking some kind of convention somewhere by doing it this way lol

Comment thread src/aotextarea.cpp Outdated
QString result = p_message.toHtmlEscaped()
.replace("\n", "<br>")
.replace(url_parser_regex, "<a href='\\1'>\\1</a>");
QString result = QString::fromStdString(this->closetags(p_message.replace("\n", "<br>").toStdString()));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

funny conversion to std:: moment, doesn't Qt have its own version of regex?
https://doc.qt.io/qt-5/qregexp.html#details

Comment thread include/aotextarea.h Outdated
void append_chatmessage(QString p_name, QString p_message,
QString p_name_colour, QString p_color = QString());
void append_error(QString p_message);
std::string closetags(std::string html);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use QString instead of std::string?

@Crystalwarrior Crystalwarrior left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, now time to test

Comment thread src/aotextarea.cpp
Comment on lines +35 to +36
QString result = this->closetags(p_message);
result = result.replace("\n", "<br>").replace(url_parser_regex, "<a href='\\1'>\\1</a>");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For url parse regex we'll need to not do that if it's already handled via html

@Crystalwarrior Crystalwarrior left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

closeTags is not doing its job at least for centering

Comment thread src/aotextarea.cpp
html = html + expected_closetag;
}
}
return html;

@Crystalwarrior Crystalwarrior Jul 31, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a qDebug() << html and none of the tags are actually closed Seems to be a centering specific issue actually. After </center> a <br> is needed

brave_6zHHsjtm6M

To properly stop centering, it should be <br><center>msg</center><br>
image

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants