-
Notifications
You must be signed in to change notification settings - Fork 10
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.
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
Do not display spoilers in message previews #13
Conversation
I seem to remember that removing the getHtmlText is fine and safe but it's been so long since I initially wrote this code that I forgot the reason. Sorry! |
The DOMParser doesn't execute styles or scripts, it's just a parser. And since only textContent leaves, which is a string displayed as a string, there's no risk of XSS or whatever. So there's no problem in removing the getHtmlText call because there's no security risk. |
Multi language should be taken care of. |
Bump! 🪨 |
Sorry, I don't know if/when I'll get the chance to make the changes you requested! That said, the actual spoiler hiding functionality continues to work well. One flaw is that (as far as I can tell??) if the function returns null it would display null in the sidebar. This seems to happen very rarely in practice (less frequently than once a week on my main account). |
Also noting that this works well with messages where the body and formatted_body text is different: a message with body like Will briefly look into localisation now. |
ae66cca
to
ee60efe
Compare
* Modify room list previewer to remove spoilers * Also use room list previewer for notification text
ee60efe
to
f9614f1
Compare
OK, updated and rebased with latest sc branch. Manual checks pass (I tested regular messages, /me, spoilers, partial spoilers, in both notification and room list preview). The null issue I mentioned earlier is now also fixed. [spoiler] has been added to the en_EN.json file. This is good for merge if you don't have any other concerns. |
Ah cool, now those errors are gone. |
* Copyright headers 1 * Licence headers 2 * Copyright Headers 3 * Copyright Headers 4 * Copyright Headers 5 * Copyright Headers 6 * Copyright headers 7 * Add copyright headers for html and config file * Replace license files and update package.json * Update with CLA * lint
"Message previews" means notifications and the room list preview. If there are more spots where message previews appear I can extend this function to cover those as well.
I can't test these changes properly right now because I forgot how to get a working build of SchildiChat. However, I tested this in the past and it worked perfectly! Hopefully it still works now that I've cherry-picked it 6 months into the future.
I want to get this into Element as well if it works well in Schildi.
As usual, thanks for everything!