Skip to content

Conversation

@samantha-lind
Copy link
Contributor

@samantha-lind samantha-lind commented Apr 25, 2025

This change extends the current HTML validation to treat liquid tags and variables differently.

We were previously replacing both of these with blanks, which was causing issues when liquid variables were used within HTML tags. We then replaced both with 'x's; however this is now causing issues with the liquid tags.

This change keeps the blanks as normal for liquid tags, but replaces the content of any variables with 'x' instead, which should deliver the result we're after.

For example
<a href="{{ optin_link.url }}" {% if optin_link.new_tab %}target="_blank" rel="noopener noreferrer"{% endif %}>
becomes:
<a href="xxxxxxxxxxxxxxxxxxx" target="_blank" rel="noopener noreferrer" >

…ently

We were previously replacing both of these with blanks, which was causing issues when liquid variables were used within HTML tags. This change keeps the blanks as normal for liquid tags, but replaces the content of any variables with 'x' instead.
# First replace liquid tags with spaces
html = html.gsub(LIQUID_TAG) { |tag| " " * tag.size }
# Then replace liquid variables with x's
html.gsub(LIQUID_VARIABLE) { |var| "x" * var.size }
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! If we're no longer using LIQUID_TAG_OR_VARIABLE think we can drop L12?

Copy link
Contributor

@eballantine eballantine left a comment

Choose a reason for hiding this comment

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

Nice one ✨
Reminder after merging to finish the steps to publish, noted in the readme.

@samantha-lind samantha-lind merged commit 02f9d39 into main Apr 25, 2025
1 check passed
@samantha-lind samantha-lind deleted the fix-html-validator-on-liquid-variable-for-tag branch April 25, 2025 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants