Skip to content

added content escaping for text nodes#158

Merged
chasefleming merged 2 commits into
chasefleming:mainfrom
whisk:main
Jul 22, 2025
Merged

added content escaping for text nodes#158
chasefleming merged 2 commits into
chasefleming:mainfrom
whisk:main

Conversation

@whisk

@whisk whisk commented Jul 12, 2025

Copy link
Copy Markdown
Contributor

Description

Text() now escapes HTML5 special characters for safe rendering. Raw() is still available if you need to render code without escaping HTML5 elements — for example, if you need non-standard tags.

Technically, only & and < need to be escaped, not >. I still escape > for consistency and the resulting code passes the validator.

Some context

I accidentally ran into the problem that elements were not escaped in Text() and learned that there was no difference between Text and Raw nodes.

Comment thread utils.go Outdated

// EscapeNodeContents escapes HTML5 special characters in a string to ensure safe rendering as a text node
func EscapeNodeContents(s string) string {
s = strings.ReplaceAll(s, "&", "&amp;")

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Same comment as here: #159 (comment)

@chasefleming chasefleming requested a review from Copilot July 15, 2025 03:56

Copilot AI 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.

Pull Request Overview

This PR ensures that Text() nodes escape HTML5 special characters by introducing a new escaping utility and updating rendering methods, while preserving Raw() for unescaped content.

  • Added EscapeNodeContents in utils.go to replace &, <, and > with their HTML entities.
  • Updated all TextNode rendering methods to call EscapeNodeContents.
  • Added unit tests in elements_test.go to verify the escaping behavior for various characters.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
utils.go New EscapeNodeContents function and strings import
elements.go Updated Text doc comment; render methods use escaping
elements_test.go New tests for basic and HTML-escaped Text rendering
Comments suppressed due to low confidence (1)

elements_test.go:692

  • Add a test case to verify that '&' characters are properly escaped to '&' in Text rendering, for example:
func TestTextWithAmpersandEscaping(t *testing.T) {
    expected := `<p>A &amp; B</p>`
    el := P(nil, Text("A & B"))
    assert.Equal(t, expected, el.Render())
}
}

Comment thread utils.go Outdated

@chasefleming chasefleming left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for adding this!

@chasefleming chasefleming merged commit 6958c58 into chasefleming:main Jul 22, 2025
1 check passed
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