[CORL-1001] Wordlist Fixes (#2920)

* fix: improve wordlist highlighting and perf

* fix: updated tests

* fix: implmeneted new regexp lib/patterns

* fix: improve comment body css

* fix: take into account the tree shaking is disabled

See: https://github.com/webpack/webpack/issues/7094

Co-authored-by: Chi Vinh Le <vinh@vinh.tech>
Co-authored-by: Kim Gardner <kgardnr@gmail.com>
This commit is contained in:
Wyatt Johnson
2020-04-06 15:04:05 -04:00
committed by GitHub
co-authored by Chi Vinh Le Kim Gardner
parent 8966a8201b
commit 6711f09a79
20 changed files with 616 additions and 185 deletions
@@ -12,12 +12,12 @@ $comment-link-active: var(--v2-palette-primary-darkest);
color: $comment-content;
overflow-wrap: break-word;
& * bold,
& * strong {
b,
strong {
font-weight: var(--v2-font-weight-primary-bold);
}
& * italic,
& * em {
i,
em {
font-style: italic;
}
blockquote {
@@ -50,3 +50,7 @@ $comment-link-active: var(--v2-palette-primary-darkest);
}
}
}
.highlight {
white-space: pre-wrap;
}
@@ -16,6 +16,7 @@ it("renders correctly", () => {
},
className: "custom",
children: "Hello <b>Bob</b>, you bad guy",
highlight: true,
};
const renderer = createRenderer();
renderer.render(<CommentContent {...props} />);
@@ -33,6 +34,26 @@ it("renders empty words correctly", () => {
},
className: "custom",
children: "Hello <b>Bob</b>, you bad guy",
highlight: true,
};
const renderer = createRenderer();
renderer.render(<CommentContent {...props} />);
expect(renderer.getRenderOutput()).toMatchSnapshot();
});
it("renders correctly even if it has consecutive banned words on comments", () => {
const props: PropTypesOf<typeof CommentContent> = {
phrases: {
locale: "en-US",
wordList: {
suspect: ["worse"],
banned: ["bad"],
},
},
className: "custom",
children:
"This is a very long comment with bad words. Let's try bad and bad. Now bad bad.\nBad BAD bad.\n",
highlight: true,
};
const renderer = createRenderer();
renderer.render(<CommentContent {...props} />);
@@ -1,7 +1,12 @@
import cn from "classnames";
import React, { FunctionComponent, useMemo } from "react";
import striptags from "striptags";
import { getPhrasesRegExp, GetPhrasesRegExpOptions } from "coral-admin/helpers";
import {
getPhrasesRegExp,
GetPhrasesRegExpOptions,
markHTMLNode,
} from "coral-admin/helpers";
import { createPurify } from "coral-common/utils/purify";
import styles from "./CommentContent.css";
@@ -15,81 +20,60 @@ interface Props {
className?: string;
children: string | React.ReactElement;
phrases: GetPhrasesRegExpOptions;
}
function escapeHTML(unsafe: string) {
return unsafe
.replace(/&/g, "&amp;")
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;")
.replace(/"/g, "&quot;")
.replace(/'/g, "&#039;");
}
// markPhrasesHTML looks for `supsect` and `banned` words inside `text` given
// the settings applied for the locale and highlights them by returning an HTML
// string.
function markPhrasesHTML(text: string, expression: RegExp) {
const tokens = text.split(expression);
if (tokens.length === 1) {
return text;
}
return tokens
.map((token, i) =>
// Using our Regexp patterns it returns tokens arranged this way
// [STRING_WITH_NO_MATCH, NEW_WORD_DELIMITER, MATCHED_WORD, ...].
// This pattern repeats throughout. Next line will mark MATCHED_WORD
// and escape all tokens.
i % 3 === 2 ? `<mark>${escapeHTML(token)}</mark>` : escapeHTML(token)
)
.join("");
}
// markHTMLNode manipulates the node by looking for #text nodes and adding markers
// for `supsectWords` and `bannedWords`.
function markHTMLNode(parentNode: Node, expression: RegExp) {
parentNode.childNodes.forEach(node => {
if (node.nodeName === "#text") {
const newContent = markPhrasesHTML(node.textContent!, expression);
if (newContent !== node.textContent) {
const newNode = document.createElement("span");
newNode.innerHTML = newContent;
parentNode.replaceChild(newNode, node);
}
} else {
markHTMLNode(node, expression);
}
});
highlight?: boolean;
}
const CommentContent: FunctionComponent<Props> = ({
phrases,
className,
children,
highlight = false,
}) => {
// Cache the expression used via memo. This will reduce duplicate renders of
// this comment content when the children change but the phrase configuration
// does not change. The regExp is already cached on a deeper level
// automatically, this is just lessening that impact further.
const expression = useMemo(() => getPhrasesRegExp(phrases), [phrases]);
const expression = useMemo(() => {
// If we aren't in highlight mode for this comment, don't even attempt to
// generate the expression.
if (!highlight) {
return null;
}
if (typeof children === "string") {
// We create a Shadow DOM Tree with the HTML body content and
// use it as a parser.
return getPhrasesRegExp(phrases);
}, [phrases, highlight]);
// Cache the parsed comment node. If the children cannot be parsed, this will
// be null.
const parsed = useMemo(() => {
if (typeof children !== "string") {
return null;
}
// Sanitize the input for display.
let html = purify.sanitize(children);
if (highlight) {
html = striptags(html, ["a"]);
}
// We create a Shadow DOM Tree with the HTML body content and use it as a
// parser.
const node = document.createElement("div");
node.innerHTML = purify.sanitize(children);
node.innerHTML = html;
// If the expression is available, then mark the nodes.
if (expression) {
// Then we traverse it recursively and manipulate it to highlight suspect words
// and banned words.
markHTMLNode(node, expression);
}
// Finally we render the content of the Shadow DOM Tree
return node;
}, [children, expression, highlight]);
if (parsed) {
return (
<div
className={cn(className, styles.root)}
dangerouslySetInnerHTML={{ __html: node.innerHTML }}
className={cn(className, styles.root, highlight && styles.highlight)}
dangerouslySetInnerHTML={{ __html: parsed.innerHTML }}
/>
);
}
@@ -2,10 +2,23 @@
exports[`renders correctly 1`] = `
<div
className="custom CommentContent-root"
className="custom CommentContent-root CommentContent-highlight"
dangerouslySetInnerHTML={
Object {
"__html": "Hello <b>Bob</b><span>, you <mark>bad</mark> guy</span>",
"__html": "<span>Hello Bob, you <mark>bad</mark> guy</span>",
}
}
/>
`;
exports[`renders correctly even if it has consecutive banned words on comments 1`] = `
<div
className="custom CommentContent-root CommentContent-highlight"
dangerouslySetInnerHTML={
Object {
"__html": "<span>This is a very long comment with <mark>bad</mark> words. Let's try <mark>bad</mark> and <mark>bad</mark>. Now <mark>bad</mark> bad.
<mark>Bad</mark> BAD <mark>bad</mark>.
</span>",
}
}
/>
@@ -13,10 +26,10 @@ exports[`renders correctly 1`] = `
exports[`renders empty words correctly 1`] = `
<div
className="custom CommentContent-root"
className="custom CommentContent-root CommentContent-highlight"
dangerouslySetInnerHTML={
Object {
"__html": "Hello <b>Bob</b>, you bad guy",
"__html": "Hello Bob, you bad guy",
}
}
/>
@@ -37,6 +37,7 @@ interface Props {
username: string;
createdAt: string;
body: string;
highlight?: boolean;
inReplyTo?: {
id: string;
username: string | null;
@@ -82,6 +83,7 @@ const ModerateCard: FunctionComponent<Props> = ({
username,
createdAt,
body,
highlight = false,
inReplyTo,
comment,
settings,
@@ -222,7 +224,11 @@ const ModerateCard: FunctionComponent<Props> = ({
)}
</div>
<div className={styles.contentArea}>
<CommentContent phrases={phrases} className={styles.content}>
<CommentContent
highlight={highlight}
phrases={phrases}
className={styles.content}
>
{commentBody}
</CommentContent>
<div className={styles.viewContext}>
@@ -215,6 +215,16 @@ const ModerateCardContainer: FunctionComponent<Props> = ({
},
[comment]
);
// Only highlight comments that have been flagged for containing a banned or
// suspect word.
const highlight = comment.revision
? comment.revision.actionCounts.flag.reasons.COMMENT_DETECTED_BANNED_WORD +
comment.revision.actionCounts.flag.reasons
.COMMENT_DETECTED_SUSPECT_WORD >
0
: false;
return (
<>
<FadeInTransition active={Boolean(comment.enteredLive)}>
@@ -227,6 +237,7 @@ const ModerateCardContainer: FunctionComponent<Props> = ({
}
createdAt={comment.createdAt}
body={comment.body!}
highlight={highlight}
inReplyTo={comment.parent && comment.parent.author}
comment={comment}
settings={settings}
@@ -296,6 +307,16 @@ const enhanced = withFragmentContainer<Props>({
statusLiveUpdated
createdAt
body
revision {
actionCounts {
flag {
reasons {
COMMENT_DETECTED_BANNED_WORD
COMMENT_DETECTED_SUSPECT_WORD
}
}
}
}
tags {
code
}
@@ -44,6 +44,7 @@ exports[`renders approved correctly 1`] = `
>
<CommentContent
className="ModerateCard-content"
highlight={false}
phrases={
Object {
"locale": "en-US",
@@ -178,6 +179,7 @@ exports[`renders correctly 1`] = `
>
<CommentContent
className="ModerateCard-content"
highlight={false}
phrases={
Object {
"locale": "en-US",
@@ -312,6 +314,7 @@ exports[`renders dangling correctly 1`] = `
>
<CommentContent
className="ModerateCard-content"
highlight={false}
phrases={
Object {
"locale": "en-US",
@@ -446,6 +449,7 @@ exports[`renders rejected correctly 1`] = `
>
<CommentContent
className="ModerateCard-content"
highlight={false}
phrases={
Object {
"locale": "en-US",
@@ -589,6 +593,7 @@ exports[`renders reply correctly 1`] = `
>
<CommentContent
className="ModerateCard-content"
highlight={false}
phrases={
Object {
"locale": "en-US",
@@ -723,6 +728,7 @@ exports[`renders story info 1`] = `
>
<CommentContent
className="ModerateCard-content"
highlight={false}
phrases={
Object {
"locale": "en-US",
@@ -895,6 +901,7 @@ exports[`renders tombstoned when comment is deleted 1`] = `
>
<CommentContent
className="ModerateCard-content"
highlight={false}
phrases={
Object {
"locale": "en-US",
@@ -1,5 +1,7 @@
import { lowerCase, uniqBy } from "lodash";
import { LanguageCode } from "coral-common/helpers";
import { createWordListRegExp } from "coral-common/utils";
import createWordListRegExp from "coral-common/utils/createWordListRegExp";
export interface GetPhrasesRegExpOptions {
locale: string;
@@ -17,20 +19,35 @@ export function getPhrasesRegExp({
return null;
}
return createWordListRegExp(locale as LanguageCode, [...banned, ...suspect]);
// Because the banned and suspect word lists may sometimes overlap, we should
// make this list as short as possible before compiling it into a RegExp.
const phrases = uniqBy<string>([...banned, ...suspect], lowerCase);
// The locale is passed down to us from the Graph, we can cast it to a
// LanguageCode.
return createWordListRegExp(locale as LanguageCode, phrases);
}
// cache is used as a global validator to the cached RegExp used by the
// Cache is used as a global validator to the cached RegExp used by the
// application. We expect that generally, there is only ever one word list used
// by the client at a time, so this ensures that we only re-create the word list
// if we must.
const cache = {
interface Cache {
keys: {
locale: string;
suspect: ReadonlyArray<string>;
banned: ReadonlyArray<string>;
};
value: RegExp | null;
}
const cache: Cache = {
keys: {
locale: "",
suspect: [] as ReadonlyArray<string>,
banned: [] as ReadonlyArray<string>,
suspect: [],
banned: [],
},
value: null as RegExp | null,
value: null,
};
export default function(options: GetPhrasesRegExpOptions) {
@@ -57,7 +74,12 @@ export default function(options: GetPhrasesRegExpOptions) {
// If the cache is expired, or the value doesn't exist, regenerate it.
if (expired) {
cache.value = getPhrasesRegExp(options);
try {
cache.value = getPhrasesRegExp(options);
} catch (err) {
window.console.error(err);
return null;
}
}
return cache.value;
+1
View File
@@ -3,3 +3,4 @@ export {
default as getPhrasesRegExp,
GetPhrasesRegExpOptions,
} from "./getPhrasesRegExp";
export { default as markHTMLNode } from "./markHTMLNode";
@@ -0,0 +1,58 @@
// markPhrasesHTML looks for `suspect` and `banned` words inside `text` given
// the settings applied for the locale and highlights them by returning an HTML
// string.
function markPhrasesHTML(text: string, expression: RegExp) {
const tokens = text.split(expression);
// If there were less than two matches, then there was no matched word
// associated with the passed in text.
if (tokens.length < 3) {
return null;
}
return tokens
.map((token, i) =>
// Using our Regexp patterns it returns tokens arranged this way:
//
// - STRING_WITH_NO_MATCH
// - NEW_WORD_DELIMITER
// - MATCHED_WORD
// - NEW_WORD_DELIMITER
// - ...
//
// This pattern repeats throughout. Next line will mark MATCHED_WORD.
i % 4 === 2 ? "<mark>" + token + "</mark>" : token
)
.join("");
}
// markHTMLNode manipulates the node by looking for #text nodes and adding
// markers.
export default function markHTMLNode(parentNode: Node, expression: RegExp) {
parentNode.childNodes.forEach(node => {
// Anchor links are already marked by default, skip them now.
if (node.nodeName === "A") {
return;
}
// If the node isn't of text type then we can't mark it directly.
if (node.nodeName !== "#text") {
return markHTMLNode(node, expression);
}
// If the node doesn't have any text content, then we can't mark it either.
if (!node.textContent) {
return;
}
// We've encountered a text node with text content that isn't in an anchor
// link. We should try to mark and replace it's content.
const replacement = markPhrasesHTML(node.textContent, expression);
if (replacement) {
// Create the new span node to replace the old node with.
const newNode = document.createElement("span");
newNode.innerHTML = replacement;
parentNode.replaceChild(newNode, node);
}
});
}
@@ -3,6 +3,10 @@ $fullscreenZIndex: 10;
.wrapper {
@mixin bodyCopy;
i
em {
font-style: italic;
}
b,
strong {
font-weight: var(--font-weight-medium);
@@ -3,12 +3,12 @@
color: var(--palette-text-dark);
overflow-wrap: break-word;
& * bold,
& * strong {
b,
strong {
font-weight: var(--font-weight-medium);
}
& * italic,
& * em {
i
em {
font-style: italic;
}
blockquote {