mirror of https://github.com/vector-im/riot-web
Fix highlights not being applied to plaintext messages (#9126)
* Fix highlights not being applied to plaintext messages * Add percy test for search result highlighting * Fix percy target * Update timeline.spec.tspull/28788/head^2
parent
a866005bea
commit
e864dbfacf
|
@ -57,12 +57,12 @@ const expectAvatar = (e: JQuery<HTMLElement>, avatarUrl: string): void => {
|
||||||
});
|
});
|
||||||
};
|
};
|
||||||
|
|
||||||
const sendEvent = (roomId: string): Chainable<ISendEventResponse> => {
|
const sendEvent = (roomId: string, html = false): Chainable<ISendEventResponse> => {
|
||||||
return cy.sendEvent(
|
return cy.sendEvent(
|
||||||
roomId,
|
roomId,
|
||||||
null,
|
null,
|
||||||
"m.room.message" as EventType,
|
"m.room.message" as EventType,
|
||||||
MessageEvent.from("Message").serialize().content,
|
MessageEvent.from("Message", html ? "<b>Message</b>" : undefined).serialize().content,
|
||||||
);
|
);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
@ -261,6 +261,18 @@ describe("Timeline", () => {
|
||||||
// Make sure "collapse" link button worked
|
// Make sure "collapse" link button worked
|
||||||
cy.get(".mx_GenericEventListSummary_toggle[aria-expanded=false]");
|
cy.get(".mx_GenericEventListSummary_toggle[aria-expanded=false]");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("should highlight search result words regardless of formatting", () => {
|
||||||
|
sendEvent(roomId);
|
||||||
|
sendEvent(roomId, true);
|
||||||
|
cy.visit("/#/room/" + roomId);
|
||||||
|
|
||||||
|
cy.get(".mx_RoomHeader_searchButton").click();
|
||||||
|
cy.get(".mx_SearchBar_input input").type("Message{enter}");
|
||||||
|
|
||||||
|
cy.get(".mx_EventTile:not(.mx_EventTile_contextual)").find(".mx_EventTile_searchHighlight").should("exist");
|
||||||
|
cy.get(".mx_RoomView_searchResultsPanel").percySnapshotElement("Highlighted search results");
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("message sending", () => {
|
describe("message sending", () => {
|
||||||
|
|
|
@ -470,40 +470,38 @@ export function bodyToHtml(content: IContent, highlights: Optional<string[]>, op
|
||||||
}
|
}
|
||||||
|
|
||||||
let strippedBody: string;
|
let strippedBody: string;
|
||||||
let safeBody: string;
|
let safeBody: string; // safe, sanitised HTML, preferred over `strippedBody` which is fully plaintext
|
||||||
let isDisplayedWithHtml: boolean;
|
|
||||||
// XXX: We sanitize the HTML whilst also highlighting its text nodes, to avoid accidentally trying
|
|
||||||
// to highlight HTML tags themselves. However, this does mean that we don't highlight textnodes which
|
|
||||||
// are interrupted by HTML tags (not that we did before) - e.g. foo<span/>bar won't get highlighted
|
|
||||||
// by an attempt to search for 'foobar'. Then again, the search query probably wouldn't work either
|
|
||||||
try {
|
try {
|
||||||
if (highlights && highlights.length > 0) {
|
|
||||||
const highlighter = new HtmlHighlighter("mx_EventTile_searchHighlight", opts.highlightLink);
|
|
||||||
const safeHighlights = highlights
|
|
||||||
// sanitizeHtml can hang if an unclosed HTML tag is thrown at it
|
// sanitizeHtml can hang if an unclosed HTML tag is thrown at it
|
||||||
// A search for `<foo` will make the browser crash
|
// A search for `<foo` will make the browser crash an alternative would be to escape HTML special characters
|
||||||
// an alternative would be to escape HTML special characters
|
// but that would bring no additional benefit as the highlighter does not work with those special chars
|
||||||
// but that would bring no additional benefit as the highlighter
|
const safeHighlights = highlights
|
||||||
// does not work with those special chars
|
?.filter((highlight: string): boolean => !highlight.includes("<"))
|
||||||
.filter((highlight: string): boolean => !highlight.includes("<"))
|
|
||||||
.map((highlight: string): string => sanitizeHtml(highlight, sanitizeParams));
|
.map((highlight: string): string => sanitizeHtml(highlight, sanitizeParams));
|
||||||
// XXX: hacky bodge to temporarily apply a textFilter to the sanitizeParams structure.
|
|
||||||
sanitizeParams.textFilter = function(safeText) {
|
|
||||||
return highlighter.applyHighlights(safeText, safeHighlights).join('');
|
|
||||||
};
|
|
||||||
}
|
|
||||||
|
|
||||||
let formattedBody = typeof content.formatted_body === 'string' ? content.formatted_body : null;
|
let formattedBody = typeof content.formatted_body === 'string' ? content.formatted_body : null;
|
||||||
const plainBody = typeof content.body === 'string' ? content.body : "";
|
const plainBody = typeof content.body === 'string' ? content.body : "";
|
||||||
|
|
||||||
if (opts.stripReplyFallback && formattedBody) formattedBody = stripHTMLReply(formattedBody);
|
if (opts.stripReplyFallback && formattedBody) formattedBody = stripHTMLReply(formattedBody);
|
||||||
strippedBody = opts.stripReplyFallback ? stripPlainReply(plainBody) : plainBody;
|
strippedBody = opts.stripReplyFallback ? stripPlainReply(plainBody) : plainBody;
|
||||||
|
|
||||||
bodyHasEmoji = mightContainEmoji(isFormattedBody ? formattedBody : plainBody);
|
bodyHasEmoji = mightContainEmoji(isFormattedBody ? formattedBody : plainBody);
|
||||||
|
|
||||||
// Only generate safeBody if the message was sent as org.matrix.custom.html
|
const highlighter = safeHighlights?.length
|
||||||
|
? new HtmlHighlighter("mx_EventTile_searchHighlight", opts.highlightLink)
|
||||||
|
: null;
|
||||||
|
|
||||||
if (isFormattedBody) {
|
if (isFormattedBody) {
|
||||||
isDisplayedWithHtml = true;
|
if (highlighter) {
|
||||||
|
// XXX: We sanitize the HTML whilst also highlighting its text nodes, to avoid accidentally trying
|
||||||
|
// to highlight HTML tags themselves. However, this does mean that we don't highlight textnodes which
|
||||||
|
// are interrupted by HTML tags (not that we did before) - e.g. foo<span/>bar won't get highlighted
|
||||||
|
// by an attempt to search for 'foobar'. Then again, the search query probably wouldn't work either
|
||||||
|
// XXX: hacky bodge to temporarily apply a textFilter to the sanitizeParams structure.
|
||||||
|
sanitizeParams.textFilter = function(safeText) {
|
||||||
|
return highlighter.applyHighlights(safeText, safeHighlights).join('');
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
safeBody = sanitizeHtml(formattedBody, sanitizeParams);
|
safeBody = sanitizeHtml(formattedBody, sanitizeParams);
|
||||||
const phtml = cheerio.load(safeBody, {
|
const phtml = cheerio.load(safeBody, {
|
||||||
|
@ -533,12 +531,14 @@ export function bodyToHtml(content: IContent, highlights: Optional<string[]>, op
|
||||||
if (bodyHasEmoji) {
|
if (bodyHasEmoji) {
|
||||||
safeBody = formatEmojis(safeBody, true).join('');
|
safeBody = formatEmojis(safeBody, true).join('');
|
||||||
}
|
}
|
||||||
|
} else if (highlighter) {
|
||||||
|
safeBody = highlighter.applyHighlights(plainBody, safeHighlights).join('');
|
||||||
}
|
}
|
||||||
} finally {
|
} finally {
|
||||||
delete sanitizeParams.textFilter;
|
delete sanitizeParams.textFilter;
|
||||||
}
|
}
|
||||||
|
|
||||||
const contentBody = isDisplayedWithHtml ? safeBody : strippedBody;
|
const contentBody = safeBody ?? strippedBody;
|
||||||
if (opts.returnString) {
|
if (opts.returnString) {
|
||||||
return contentBody;
|
return contentBody;
|
||||||
}
|
}
|
||||||
|
@ -576,11 +576,11 @@ export function bodyToHtml(content: IContent, highlights: Optional<string[]>, op
|
||||||
});
|
});
|
||||||
|
|
||||||
let emojiBodyElements: JSX.Element[];
|
let emojiBodyElements: JSX.Element[];
|
||||||
if (!isDisplayedWithHtml && bodyHasEmoji) {
|
if (!safeBody && bodyHasEmoji) {
|
||||||
emojiBodyElements = formatEmojis(strippedBody, false) as JSX.Element[];
|
emojiBodyElements = formatEmojis(strippedBody, false) as JSX.Element[];
|
||||||
}
|
}
|
||||||
|
|
||||||
return isDisplayedWithHtml ?
|
return safeBody ?
|
||||||
<span
|
<span
|
||||||
key="body"
|
key="body"
|
||||||
ref={opts.ref}
|
ref={opts.ref}
|
||||||
|
|
Loading…
Reference in New Issue