Skip to content
Merged
Show file tree
Hide file tree
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
57 changes: 55 additions & 2 deletions apps/web/src/Notifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,59 @@
},
};

/**
* Extracts plain text from a message body, replacing any spoilered content
* with '[Spoiler]' to prevent spoilers in desktop notifications.
*/
function getNotificationBodyWithoutSpoilers(ev: MatrixEvent): string {
const content = ev.getContent();
const plainBody = content.body ?? "";
const formattedBody = content.formatted_body;

if (typeof formattedBody !== "string" || !formattedBody.length) {
return plainBody;
}

/** Recursively walks HTML tree to hide spoilers. */
function replaceSpoilers(node: Node): Node {

Check warning on line 98 in apps/web/src/Notifier.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Move function 'replaceSpoilers' to the outer scope.

See more on https://sonarcloud.io/project/issues?id=element-web&issues=AZ13nQ--f_mKNuNMilDF&open=AZ13nQ--f_mKNuNMilDF&pullRequest=31699
if (node.nodeType !== Node.ELEMENT_NODE || !(node instanceof Element)) {
return node;
}

if (node.hasAttribute("data-mx-spoiler")) {

Check failure on line 103 in apps/web/src/Notifier.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Prefer `.dataset` over `hasAttribute(…)`.

See more on https://sonarcloud.io/project/issues?id=element-web&issues=AZ13nQ--f_mKNuNMilDG&open=AZ13nQ--f_mKNuNMilDG&pullRequest=31699
const e = document.createElement("span");
e.appendChild(document.createTextNode("[Spoiler]"));
return e;
}

for (const childNode of node.childNodes) {
node.replaceChild(replaceSpoilers(childNode), childNode);
}

return node;
}

try {
// Dev note: ideally we would reuse more of the existing rendering stack
// rather than re-parsing and updating the generated HTML here. However,
// that rendering stack is currently quite consolidated and cannot
// easily be refactored to allow the call-site to control how spoilers
// are rendered. The problem is that we now need two different output
// formats:
// - The existing format where spoilers are wrapped in html <span> tags
// - The new format where the spoilered text is replaced with [Spoiler]

const parser = new DOMParser();
Comment thread
t3chguy marked this conversation as resolved.
const doc = parser.parseFromString(formattedBody, "text/html");

// Use textContent rather than innerHTML/outerHTML since textContent is
// XSS-safe and the input is untrusted.
return replaceSpoilers(doc.body).textContent ?? plainBody;
} catch {
return plainBody;
}
}

export const enum NotifierEvent {
NotificationHiddenChange = "notification_hidden_change",
}
Expand Down Expand Up @@ -134,7 +187,7 @@
// notificationMessageForEvent includes sender, but we already have the sender here
const msgType = ev.getContent().msgtype;
if (ev.getContent().body && (!msgType || !msgTypeHandlers.hasOwnProperty(msgType))) {
msg = stripPlainReply(ev.getContent().body);
msg = stripPlainReply(getNotificationBodyWithoutSpoilers(ev));
}
} else if (ev.getType() === "m.room.member") {
// context is all in the message here, we don't need
Expand All @@ -145,7 +198,7 @@
// notificationMessageForEvent includes sender, but we've just out sender in the title
const msgType = ev.getContent().msgtype;
if (ev.getContent().body && (!msgType || !msgTypeHandlers.hasOwnProperty(msgType))) {
msg = stripPlainReply(ev.getContent().body);
msg = stripPlainReply(getNotificationBodyWithoutSpoilers(ev));
}
}

Expand Down
40 changes: 40 additions & 0 deletions apps/web/test/unit-tests/Notifier-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,46 @@ describe("Notifier", () => {
reply,
);
});

it.each([
["This was a triumph", "This was a triumph", "This was a triumph"],
["This was a triumph", "<span data-mx-spoiler>This was a triumph</span>", "[Spoiler]"],
["This was a triumph", '<span data-mx-spoiler="triumph">This was a triumph</span>', "[Spoiler]"],
["foo bar baz", "foo <span data-mx-spoiler>bar</span> baz", "foo [Spoiler] baz"],
["foo foo foo", "foo <span data-mx-spoiler>foo</span> foo", "foo [Spoiler] foo"],
[
"a b c d e",
"a <span data-mx-spoiler>b</span> c <span data-mx-spoiler>d</span> e",
"a [Spoiler] c [Spoiler] e",
],
["foo foo", "foo <span data-mx-spoiler></span> foo", "foo [Spoiler] foo"],
["foo bar baz", "foo <span data-mx-spoiler>b<em>a</em>r</span> baz", "foo [Spoiler] baz"],
["foobar", "<span data-mx-spoiler>foo</span><span data-mx-spoiler>bar</span>", "[Spoiler][Spoiler]"],
["foo bar baz", "<strong>foo <span data-mx-spoiler>bar</span> baz</strong>", "foo [Spoiler] baz"],
["foo <bar> baz", "foo <span data-mx-spoiler>&lt;bar&gt;</span> baz", "foo [Spoiler] baz"],
["foo\nbar\nbaz", "foo<span data-mx-spoiler><br>bar<br></span>baz", "foo[Spoiler]baz"],
])("should hide spoilers in notification", (body, formattedBody, expected) => {
const spoilerEvent = mkEvent({
event: true,
type: EventType.RoomMessage,
user: mockClient.getSafeUserId(),
room: testRoom.roomId,
content: {
msgtype: MsgType.Text,
body: body,
format: "org.matrix.custom.html",
formatted_body: formattedBody,
},
});
Notifier.displayPopupNotification(spoilerEvent, testRoom);
expect(MockPlatform.displayNotification).toHaveBeenCalledWith(
"@bob:example.org (!room1:server)",
expected,
expect.any(String),
testRoom,
spoilerEvent,
);
});
});

describe("getSoundForRoom", () => {
Expand Down
Loading