Skip to content

Hide spoilers from desktop notifications #31699

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JeftavanderHorst wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Hide spoilers from desktop notifications #31699

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1dbe910
Hide spoilers from desktop notifications
JeftavanderHorst Jan 11, 2026
bb94277
Replace unicode blocks with spoiler tag
JeftavanderHorst Jan 12, 2026
746d6ed
Merge branch 'element-hq:develop' into notification-spoilers
JeftavanderHorst Jan 26, 2026
690d2d9
Run prettier
JeftavanderHorst Jan 26, 2026
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
45 changes: 43 additions & 2 deletions src/Notifier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,47 @@ const msgTypeHandlers: Record<string, (event: MatrixEvent) => string | null> = {
},
};

/**
* 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 walk HTML tree to hide spoilers
function replaceSpoilers(node: Node): Node {
if (node.nodeType !== Node.ELEMENT_NODE || !(node instanceof Element)) {
return node;
}

if (node.hasAttribute("data-mx-spoiler")) {
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 {
const parser = new DOMParser();
Copy link
Member

@t3chguy t3chguy Jan 16, 2026

Choose a reason for hiding this comment

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

I think this needs to use more of the same rendering stack we use for the timeline to sanitise the HTML before processing

Copy link
Author

@JeftavanderHorst JeftavanderHorst Jan 23, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

My (admittedly limited) understanding is that the existing rendering stack is quite consolidated: event goes in, rendered html comes out. The problem is that we now need two different output formats:

  • The existing format where spoilers are wrapped in <span data-mx-spoiler> tags containing the actual spoilered text
  • The new format where the spoilered text is replaced with [Spoiler]

I see roughly four different solutions:

  • Break up the consolidated rendering stack to be more modular, so that call sites have more control over the ouput format. This seems more complex than what I'm comfortable with as an external contributor.
  • Add a boolean parameter to the existing rendering function, to switch between behaviors.
  • Remove the spoilers from the generated html without actually parsing html, using replaceAll or (God forbid) some regex-based solution.

None of these are appealing to me, which is why I chose approach I ended up with. It's quite possible I'm missing an obvious solution, though, since I'm not familiar with the code base. What are your thoughts?

Copy link
Member

@t3chguy t3chguy Jan 30, 2026

Choose a reason for hiding this comment

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

I think it could do with some comments around the justification, and that only the textContent is seemingly safe to use, to avoid xss injection attacks given the untrusted input html

const doc = parser.parseFromString(formattedBody, "text/html");
return replaceSpoilers(doc.body).textContent ?? plainBody;
} catch {
return plainBody;
}
}

export const enum NotifierEvent {
NotificationHiddenChange = "notification_hidden_change",
}
Expand Down Expand Up @@ -134,7 +175,7 @@ class NotifierClass extends TypedEventEmitter<keyof EmittedEvents, EmittedEvents
// 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 +186,7 @@ class NotifierClass extends TypedEventEmitter<keyof EmittedEvents, EmittedEvents
// 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
36 changes: 36 additions & 0 deletions test/unit-tests/Notifier-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,42 @@ 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