Skip to content

Raw welcome message new#13

Open
akrherz wants to merge 4 commits into
igniterealtime:mainfrom
akrherz:raw-welcome-message-new
Open

Raw welcome message new#13
akrherz wants to merge 4 commits into
igniterealtime:mainfrom
akrherz:raw-welcome-message-new

Conversation

@akrherz

@akrherz akrherz commented Oct 30, 2020

Copy link
Copy Markdown
Member

Obsoletes #12

@nsobadzhiev nsobadzhiev left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good as far as I remember what I did here. Did you test it?

Comment thread src/java/org/jivesoftware/openfire/plugin/RegistrationPlugin.java Outdated
@akrherz

akrherz commented Nov 1, 2020

Copy link
Copy Markdown
Member Author

Did you test it?

I did not, my focus was to fix the PR after I rewrote the repo git history.

<% if (errors.containsKey("missingWelcomeMessage")) { %>
<span class="jive-error-text"><br><fmt:message key="registration.props.form.welcome_message_missing" /></span>
<% } %>
<td width="95%"><textarea cols="45" rows="5" wrap="virtual" name="welcomemessage"><%= welcomeMessage %></textarea></td>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These <%= outputs need to be properly escaped, to avoid XSS.

@guusdk guusdk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The admin console page modifications need some escaping.

A changelog item is also desirable.

@akrherz

akrherz commented Feb 6, 2021

Copy link
Copy Markdown
Member Author

@nsobadzhiev are you able to address these review changes? You likely will want to create a new branch and redo your commits, or cherry pick my branch commits and then add yours ontop. I can help you with the git commands, if you need help.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants