Skip to content

Comments: Allow the Notes @mention chip markup in comment content (kses)#12503

Open
adamsilverstein wants to merge 18 commits into
WordPress:trunkfrom
adamsilverstein:add/notes-mention-kses
Open

Comments: Allow the Notes @mention chip markup in comment content (kses)#12503
adamsilverstein wants to merge 18 commits into
WordPress:trunkfrom
adamsilverstein:add/notes-mention-kses

Conversation

@adamsilverstein

@adamsilverstein adamsilverstein commented Jul 13, 2026

Copy link
Copy Markdown
Member

Backports the mention kses allowance from the Gutenberg Notes @mention work:

The notification layer (from the still-open WordPress/gutenberg#79606) has been split out to #12548 with its own Trac ticket, so this PR can land as soon as it is reviewed without waiting on the upstream notification work.

What

The Notes @ mention completer stores a mention as a non-interactive chip carrying the mentioned user's ID in a class token:

<span class="wp-note-mention user-N">@Name</span>

This PR allows exactly that markup - and nothing more - through comment kses, via two small always-on filters (hooked in default-filters.php):

  • _wp_kses_allow_note_mention_span() (new, private) extends the pre_comment_content allowlist with class on span elements.
  • _wp_kses_sanitize_note_mention_classes() (new, private) runs on pre_comment_content at priority 11 - right after wp_filter_kses - and uses the HTML API (class_list() / remove_class()) to strip every span class token except wp-note-mention and user-N. It bails while wp_filter_kses is not attached, so users with unfiltered_html (filtered through wp_filter_post_kses, where arbitrary classes are already permitted) are never narrowed below what core allows them.

wp_filter_comment() is untouched: no per-comment-type arming or disarming of kses state.

Why

This follows the discussion on this PR and on WordPress/gutenberg#80496:

  • @westonruter's concern about the original approach: allowing an open-ended class on note links is a live CSS/JS selector surface in the admin. His data-attribute suggestion turned out to need a kses allowance anyway (data-* is not allowed by default in the comment context), and per @Mamaduka's review, an anchor-based mention breaks in the Link format UI regardless of how the ID is carried - mentions aren't links, they're tokens rendered as chips.
  • The resolution keeps the class (it doubles as the rich-text format-differentiation and styling hook) but closes the open-class hole a different way: the allowance is unconditional yet reduced to exactly two inert tokens on a semantics-free element, so there is nothing left to scope per comment type and the stateful arm/disarm machinery disappears.

Behavior change: all commenters (including anonymous ones) can persist <span> elements whose class is limited to wp-note-mention and/or user-N. This is inert - the tokens are not styling or scripting hooks outside the notes sidebar, and mention notification parsing (#12548) only processes note-type comments.

Testing

Unit tests in tests/phpunit/tests/kses.php:

  • test_wp_kses_allowed_html_pre_comment_content_allows_only_the_mention_span verifies the context gains the span allowance and nothing else beyond the stock $allowedtags.
  • test_note_mention_markup_survives_note_content_sanitization / test_note_mention_markup_survives_regular_comment_content_sanitization verify the chip survives for both comment types.
  • test_note_mention_span_classes_are_reduced_to_the_mention_tokens verifies junk class tokens are stripped.
  • test_note_mention_class_attribute_removed_when_no_tokens_remain verifies the class attribute is dropped entirely when no valid tokens remain.
  • test_note_mention_allows_only_class_on_mention_spans verifies every other span attribute (onclick, style, data-*, id) is still stripped.
  • test_class_is_still_stripped_from_links_in_comment_content verifies links keep the default sanitization.
  • test_note_mention_class_reduction_skipped_when_restrictive_kses_is_inactive verifies the reduction never narrows unfiltered_html users.
npm run test:php -- tests/phpunit/tests/kses.php

Trac ticket: https://core.trac.wordpress.org/ticket/65622


Proposed commit message

Comments: Allow the Notes @mention chip markup in comment content.

The Notes @mention completer stores a mention as a non-interactive chip,
`<span class="wp-note-mention user-N">@Name</span>`, the `user-N` class
token carrying the mentioned user's ID. The default comment kses allowlist
does not permit `span`, so for users without the `unfiltered_html`
capability the mention markup is stripped when the note is saved.

Allow `span` with `class` in the comment kses context, and reduce span
classes to the two mention tokens in a companion `pre_comment_content`
pass running right after kses, so the allowance stays inert: no other
element, attribute, or class token is newly available to commenters.

See related Gutenberg pull requests: https://github.com/WordPress/gutenberg/pull/79604 and https://github.com/WordPress/gutenberg/pull/80528.

Props mamaduka, westonruter.
Fixes #65622.

The notes @-mention completer stores a mention as
`<a class="wp-note-mention" data-user-id="N" href="...">@name</a>`.
The default comment kses allowlist only keeps `href` and `title` on
links, so for users without `unfiltered_html` the attributes that make
a mention a mention (the chip class and the mentioned user's ID) are
stripped on save.

Add a 'pre_comment_content' context to wp_kses_allowed_html() that
allows `class` and `data-user-id` on links so saved mentions survive
sanitization. Both attributes are inert markup.

Backports the PHP changes from the Gutenberg mentions PR:
WordPress/gutenberg#79604
@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props adamsilverstein, westonruter, wildworks, mamaduka.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions

Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

Comment thread src/wp-includes/kses.php Outdated
Allowing class and data-user-id on links in the pre_comment_content
context loosened sanitization for every comment, including anonymous
front-end comments. Both attributes are CSS/JS selector hooks, so that
would let any commenter publish links styled by theme classes or
reachable by delegated script handlers.

Attach the extended allowlist in wp_filter_comment() only while a note
comment's content is being filtered instead. Notes can only be written
by logged-in users who can edit the post and never render on the front
end, and the sanitization of regular comments is unchanged.
Design review on the Gutenberg side changed the stored mention from a
link to a plain span, since a mention marks a person rather than
offering navigation. Allow span.class and span.data-user-id instead of
the link attributes; the allowance is still attached only while a note
comment is filtered.
… attribute

The notes mention completer now stores a mention as
`<span class="wp-note-mention user-N">@name</span>`, so the kses
allowance for note comments shrinks to the single `class` attribute
and no longer needs `data-user-id`. Add a test asserting that any
other attribute is still stripped from note spans.

Trac ticket: https://core.trac.wordpress.org/ticket/65622
@adamsilverstein
adamsilverstein force-pushed the add/notes-mention-kses branch from 1fc1250 to f74c68b Compare July 13, 2026 18:26
Mentions are now inserted as links to the mentioned user's author page
- <a class="wp-note-mention user-N" href="..."> - instead of spans, so
the note allowance moves from span.class to a.class. Note content now
also picks up wp_rel_ugc()'s rel="nofollow ugc" like any other comment
link, which the tests cover with a deterministic external href.
@adamsilverstein adamsilverstein self-assigned this Jul 14, 2026
@adamsilverstein adamsilverstein added the Gutenberg Sync Pull requests syncing changes from WordPress/Gutenberg. label Jul 14, 2026
@adamsilverstein
adamsilverstein requested a review from t-hamano July 14, 2026 15:51
@adamsilverstein adamsilverstein changed the title Comments: allow note mention attributes in comment content Comments: Support Notes @mentions: kses allowance and notifications Jul 14, 2026
@Mamaduka

Copy link
Copy Markdown
Member

Sorry, only have time for a surface review.

  • Not sure if we need new methods like wp_get_note_thread_root_id. Values can be easily derived.
  • The follower notifications seem like work for a separate PR. I don't think it should be in scope for mention notifications.

I think the general API for managing email (or other) notifications would be better for the project than handling each case individually.

@adamsilverstein
adamsilverstein force-pushed the add/notes-mention-kses branch from dc57c7f to e31873f Compare July 15, 2026 20:23
@adamsilverstein adamsilverstein changed the title Comments: Support Notes @mentions: kses allowance and notifications Comments: Allow Notes @mention attributes in comment content (kses) Jul 15, 2026
@adamsilverstein

Copy link
Copy Markdown
Member Author

Sorry, only have time for a surface review.

  • Not sure if we need new methods like wp_get_note_thread_root_id. Values can be easily derived.
  • The follower notifications seem like work for a separate PR. I don't think it should be in scope for mention notifications.

I think the general API for managing email (or other) notifications would be better for the project than handling each case individually.

Reducing this to the kses only changes which matches the merged PRs. Opening a follow up for the notifications part to dig deeper.

@adamsilverstein

Copy link
Copy Markdown
Member Author

Follow up: #12548

Comment thread src/wp-includes/kses.php Outdated
Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com>

@westonruter westonruter 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.

I wonder, instead of relying on classes, would it have been better to use a data attribute instead? It could be like data-wp-note-mention-user="N". This would be less of a "blast radius" for allowing class names on notes, since in the admin allowing any class could produce undesirable results. Like someone could craft a link that is using some class that makes it look weird or maybe something malicious. In contrast, if a data attribute were used, it would be allowed by Kses by default so there would be nothing special needed. This entire PR wouldn't be necessary then.

@westonruter westonruter 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.

@adamsilverstein

Copy link
Copy Markdown
Member Author

I wonder, instead of relying on classes, would it have been better to use a data attribute instead? It could be like data-wp-note-mention-user="N". This would be less of a "blast radius" for allowing class names on notes, since in the admin allowing any class could produce undesirable results. Like someone could craft a link that is using some class that makes it look weird or maybe something malicious. In contrast, if a data attribute were used, it would be allowed by Kses by default so there would be nothing special needed. This entire PR wouldn't be necessary then.

Hmmm, thats a great suggestion and we can still change this in beta since we just introduced this feature.

When we added this originally some commits explored other attributes to use that would already be accepted by kses to avoid having to add this increased permission. I think the PR went with classes in part because it offers styling for free and also... a data attribute may not have been considered.

Let me explore that suggested in a Gutenberg PR and maybe we can eliminate the need for this PR entirely as you suggested. We could still add styles dynamically if we need them to style the mentions (admin or front end).

@Mamaduka

Copy link
Copy Markdown
Member

Left to comment in WordPress/gutenberg#80496 (comment).

It's not hard to switch between implementation details, but unless we have clear goals: code, UI, UX, we might end up going in circles.

Match the reworked Gutenberg approach (WordPress/gutenberg#80528): mentions
are span.wp-note-mention.user-N chips rather than links, so the allowance
becomes two always-on filters - span.class in the comment kses context plus
a post-kses pass reducing span classes to the two mention tokens - and the
per-note arming inside wp_filter_comment() is no longer needed.
Copilot AI review requested due to automatic review settings July 21, 2026 15:56
@adamsilverstein adamsilverstein changed the title Comments: Allow Notes @mention attributes in comment content (kses) Comments: Allow the Notes @mention chip markup in comment content (kses) Jul 21, 2026
@adamsilverstein

Copy link
Copy Markdown
Member Author

Updated in 69ec26b to match the approach that landed from the discussion here and on gutenberg#80496 / gutenberg#80528:

  • Mentions are now non-interactive <span class="wp-note-mention user-N"> chips - per @Mamaduka, mentions aren't links, and an anchor-based mention breaks in the Link format UI regardless of how the ID is carried.
  • @westonruter's open-class concern is addressed without the data attribute (which needs a kses allowance in the comment context anyway): the allowance is now always-on but a companion pre_comment_content pass reduces span classes to exactly the two mention tokens, using the HTML API. The wp_filter_comment() arm/disarm is gone entirely - comment.php is untouched.
  • The Gutenberg plugin copy disables itself when core's _wp_kses_allow_note_mention_span() exists.

Local runs: kses 366/366, comment group 582/582, REST comments controller 200/200.

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 updates WordPress core comment KSES handling to allow the Notes @mention “chip” markup (<span class="wp-note-mention user-N">…</span>) to persist in comment content while keeping the allowance inert by stripping all non-mention class tokens immediately after KSES runs.

Changes:

  • Extend the pre_comment_content KSES allowlist to permit span with a class attribute.
  • Add a post-KSES sanitization pass that reduces all span class tokens to only wp-note-mention and user-N.
  • Add PHPUnit coverage validating the allowlist change, class-token reduction behavior, and non-expansion to other elements/attributes.

Reviewed changes

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

File Description
src/wp-includes/kses.php Adds the narrow allowlist extension for span[class] and the companion sanitizer that strips all non-mention class tokens from spans.
src/wp-includes/default-filters.php Wires the new allowlist and sanitizer into core defaults (wp_kses_allowed_html and pre_comment_content).
tests/phpunit/tests/kses.php Adds unit tests ensuring the allowance is minimal and that mention markup survives/comment content stays appropriately sanitized.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/phpunit/tests/kses.php Outdated
Comment on lines +644 to +650
// The HTML API leaves the removed attribute's surrounding whitespace
// in place, hence `<span >`.
$this->assertSame(
'Hello <span >there</span>!',
wp_unslash( $filtered['comment_content'] ),
'A span with no valid mention tokens should lose its class attribute entirely.'
);
The HTML API's whitespace handling when removing the final attribute is
not part of its contract, so asserting the exact '<span >' spacing is
brittle.
Copilot AI review requested due to automatic review settings July 21, 2026 16:07
@adamsilverstein

Copy link
Copy Markdown
Member Author

Copilot's whitespace-brittleness point on the <span > assertion was valid - fixed in 5000cb4 using assertEqualHTML(), and mirrored to the Gutenberg PR.

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread src/wp-includes/kses.php
Comment on lines +1200 to +1204
$unslashed = wp_unslash( $content );

if ( ! str_contains( $unslashed, '<span' ) ) {
return $content;
}

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.

Comment on lines +617 to +626
$content = 'Hello <span class="wp-note-mention user-2 is-destructive components-button">@admin</span>!';
$filtered = wp_filter_comment( wp_slash( $this->get_mention_commentdata( 'note', $content ) ) );

remove_filter( 'pre_comment_content', 'wp_filter_kses' );

$this->assertSame(
'Hello <span class="wp-note-mention user-2">@admin</span>!',
wp_unslash( $filtered['comment_content'] ),
'Class tokens beyond `wp-note-mention` and `user-N` should be stripped from spans.'
);
Match westonruter's review on WordPress/gutenberg#80528: type the kses
filter and sanitizer signatures, and drop the try/finally filter
restoration in the test since the framework restores filters after each
test.
Copilot AI review requested due to automatic review settings July 21, 2026 16:57

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/wp-includes/kses.php:1207

  • The early bailout str_contains( $unslashed, '<span' ) is case-sensitive. A commenter could use <SPAN class="..."> so this function returns early and never strips class tokens, leaving an open-ended class attribute on spans (since class is now allowed through KSES). Removing this check avoids the bypass and matches the intended “always reduce span classes” behavior.
	$unslashed = wp_unslash( $content );

	if ( ! str_contains( $unslashed, '<span' ) ) {
		return $content;
	}

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

Labels

Gutenberg Sync Pull requests syncing changes from WordPress/Gutenberg.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants