Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Automatically decode html entities #240

Open
arcanis opened this issue May 28, 2016 · 9 comments
Open

Automatically decode html entities #240

arcanis opened this issue May 28, 2016 · 9 comments
Assignees
Labels
enhancement legacy pertaining to version 0.7 or below next_release this issue is accepted for inclusion in the next release of botkit Slack-related

Comments

@arcanis
Copy link

arcanis commented May 28, 2016

Shouldn't messages received from Slack be automatically html-decoded ?

@jlsjonas
Copy link

jlsjonas commented May 31, 2016

I agree, I now have to replace & to & (and in principle the same with < & > ) for url to properly parse the url; alternatively provide a clean() method (in order to decode only when required)

@stale
Copy link

stale bot commented Oct 22, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 22, 2018
@jlsjonas
Copy link

2 years later 😅 unless they are meanwhile? (mainly using Mattermost & Telegram currently)

@stale stale bot removed the stale label Oct 23, 2018
@benbrown benbrown self-assigned this Oct 23, 2018
@benbrown
Copy link
Contributor

@jlsjonas thanks for the input -- did you see this when using the Event API or the RTM connection? Or both?

@benbrown benbrown added the next_release this issue is accepted for inclusion in the next release of botkit label Oct 23, 2018
@jlsjonas
Copy link

If I remember correctly the project never used the RTM connection; so event API. Both might be affected though (untested)

@benbrown
Copy link
Contributor

OK yes, I can confirm that the entities come through encoded with the events api.

Now the question is, should this happen by default, or via an optional middleware?

@benbrown
Copy link
Contributor

This little snippet of code will do the job, but since it requires a new dependency that would require extra work to keep up to date, I'm not sure if we should include this in core. THOUGHTS?

const Entities = require('html-entities').AllHtmlEntities; 
const entities = new Entities();
// Decode HTML-encoded entities in Slack messages
controller.middleware.receive.use((bot, message, next) => {
  if (bot.type === 'slack' && message.text) {
     message.text = entities.decode(message.text);
  }
  next();  
});

@jlsjonas
Copy link

I would vouch to add it to core to remain consistent.

Or alternatively put it in the documentation as a note to using the Slack events API? do you have any stats regarding usage (which could help decide between the 2 options)?

Otoh, if someone creates something based on botkit and doesn't know about the difference for Slack events API this might cause issues that could only (relatively) be solved upstream to them.

Eitherway I would use https://www.npmjs.com/package/entities instead though; as html-entities seems to be no longer maintained (looking at the open issues/pr's)

@benbrown
Copy link
Contributor

Makes sense. Thanks for the recommendation on a better entity package.

@benbrown benbrown added the legacy pertaining to version 0.7 or below label May 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement legacy pertaining to version 0.7 or below next_release this issue is accepted for inclusion in the next release of botkit Slack-related
Projects
None yet
Development

No branches or pull requests

4 participants