HubSpot / draft-convert

Extensibly serialize & deserialize Draft.js ContentState with HTML.
Apache License 2.0
483 stars 94 forks source link

Possible bug in the readme example (entityToHTML) #179

Open Nasicus opened 4 years ago

Nasicus commented 4 years ago

The readme has the following example:

  entityToHTML: (entity, originalText) => {
    if (entity.type === 'LINK') {
      return <a href={entity.data.url}>{originalText}</a>;
    }
    return originalText;
  }

This works fine, however the originalText appears to be already HTML encoded and therefore if it contains special characters like an ' it will be double encoded.

E.g. originalText = my&#x27;url will result in the HTML <a href="https://google.ch">my&amp;#x27;url</a>

As a workaround I changed the function to that:

    if (entity.type === 'LINK') {
      return <a href={entity.data.url} />;
    }

    return originalText;

Now the resulting HTML will be <a href="https://google.ch">my&#x27;url</a> (because of the way the getElementHTML function works).

Not sure if this workaround is correct and can be used like this but it appears to work. I guess if the getElementHTML will never change I can leave my workaround...

Also not sure if I'm doing something wrong or if this is an actual bug.

karldanninger commented 3 years ago

Experienced this recently.

@Nasicus Your workaround works, thank you! Yet, this makes me a little uncomfortable since I'm unsure why this works the way it does.

Is there any way to not encode originalText supplied to entityToHtml? Perhaps that would defeat the purpose of the function since it's goal is to convert an entity to encoded valid HTML... 🤔

hgezim commented 3 years ago

@karldanninger

I looked into why this works and let me see if I understand it:

In entityToHTML, even when you return only <a href='...' />,

When that is converted to HTML, the above is only used to generate the tags (line 35 https://github.com/HubSpot/draft-convert/blob/1b78ab19250f4d558d8d5ea4fb0c83f121cc395f/src/blockEntities.js#L35 ).

In line 36 ( https://github.com/HubSpot/draft-convert/blob/1b78ab19250f4d558d8d5ea4fb0c83f121cc395f/src/blockEntities.js#L36 ), getElementHTML is called with originalText which actually ends up being used as the child node of a tag:

Xnip2021-05-31_21-28-15

Nasicus commented 3 years ago

@karldanninger @hgezim thanks. We're using my "workaround" in production since bascially I wrote this post. No issues.

The only thing I'm a little afraid is that the suddenly change this in the future ;)

karldanninger commented 3 years ago

@Nasicus Haha! I have that exact same fear - but we've made a note of this issue in our repo. 👍

Thank you for the explanation @hgezim !!!