Skip to content
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

Markdown: alert words highlighting requires page refresh to update existing messages. #9585

Open
brickbite opened this issue May 30, 2018 · 2 comments

Comments

@brickbite
Copy link
Collaborator

Currently, highlighting for alert words in existing messages are only updated on page refresh. Updating alert words does not update the highlighting of the content you are already viewing, but it will highlight subsequent (newly created) messages that contain alert words. This makes it possible to have messages both highlighted and not highlighted when they contain the alert words text.

Also, even when the page is refreshed, it's possible that some messages aren't highlighted (See screenshots below).

Steps to reproduce:

  1. Edit alert words.
  2. Enter new message.
  3. Repeat Step 1 (without refreshing the page)

Screenshots:

Without refreshing, alert words are removed and added using the above steps. It's possible to have different highlighting for messages with the same content:

image


Finally, setting "aaron sleeps" as the alert word, and refreshing the page. Also note that some messages still don't end up highlighted. After page refresh:

image


Related issue: duplicate alert words can be added: the check for unique alert words is probably checking string equivalence. Here we are able to add variations of "aaron sleeps".

'aaron sleeps' === 'AARON SLEEPS' is false, so it's not detected as a duplicate. At the end, note the attempt to add an existing phrase. 'aaron SLEEPS' === 'aaron SLEEPS' is true, so it is not added and we get the notice in red.

image

@zulipbot
Copy link
Member

Hello @zulip/server-markdown members, this issue was labeled with the "area: markdown" label, so you may want to check it out!

@timabbott
Copy link
Sponsor Member

The duplicated alert words bug is now fixed in 8e5b035.

For the live-update issue, I think there's a reasonable way to fix it, namely doing a full rerender of all visible messages (after calling exports.process_message on every message in message_store) after a change in the set of alert_words. I'm not sure if it's worth writing the code to do that given that folks change alert words relatively infrequently, but we could do that.

(There's also some question about what semantics are correct; the current model is that a message is treated as "alerted" only if it had an alert word using the alert words available when it was sent, not now). If we don't change those semantics, I'm not convinced it's possible to make the situation any better than it is now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants