Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 16 additions & 13 deletions src/js/_enqueues/wp/sanitize.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,25 @@
* @return {string} Stripped text.
*/
stripTags: function( text ) {
let _text = text || '';
const domParser = new DOMParser();
const htmlDocument = domParser.parseFromString(
text,
'text/html'
);

// Do the search-replace until there is nothing to be replaced.
do {
// Keep pre-replace text for comparison.
text = _text;

// Do the replacement.
_text = text
.replace( /<!--[\s\S]*?(-->|$)/g, '' )
.replace( /<(script|style)[^>]*>[\s\S]*?(<\/\1>|$)/ig, '' )
.replace( /<\/?[a-z][\s\S]*?(>|$)/ig, '' );
} while ( _text !== text );
/*
* The following self-assignment appears to be a no-op, but it isn't.
* It enforces the escaping. Reading the `innerText` property decodes
* character references, returning a raw string. When written, however,
* the text is re-escaped to ensure that the rendered text replicates
* what it's given.
*
* See <https://github.com/WordPress/wordpress-develop/pull/10536#discussion_r2550615378>.
*/
htmlDocument.body.innerText = htmlDocument.body.innerText;
Copy link
Member

Choose a reason for hiding this comment

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

A way to get around the PhpStorm warning:

Suggested change
htmlDocument.body.innerText = htmlDocument.body.innerText;
htmlDocument.body.innerText = String( htmlDocument.body.innerText );

Copy link
Member

Choose a reason for hiding this comment

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

what if we just ignore it or file a bug report with Jetbrains? PhpStorm is wrong here because it doesn’t understand the code. Seems silly to me to modify our code to make up for that. it’s just a warning, and hopefully any reviewer will not the comment explaining why it’s there, as a guard against breaking this by “fixing” it.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we could go either way. I just don't have a lot of faith in Jetbrains fixing this issue (or others I've filed) in a timely manner.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

before we merge, do we need to check if any minifiers might also make this mistake and remove the line?

if that’s a hard question to answer, we can merge and see. it’s early in the release cycle so we should have ample time to test

Copy link
Member

Choose a reason for hiding this comment

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


// Return the text with stripped tags.
return _text;
return htmlDocument.body.innerHTML;
},

/**
Expand Down
Loading