springload / draftjs_exporter

Convert Draft.js ContentState to HTML
https://www.draftail.org/blog/2018/03/13/rethinking-rich-text-pipelines-with-draft-js
MIT License
83 stars 21 forks source link

Entities with adjacent offset are rendered incorrectly #106

Closed ericpai closed 6 years ago

ericpai commented 6 years ago

Test version: v2.1.4

Test with the following code:

from draftjs_exporter.constants import BLOCK_TYPES
from draftjs_exporter.constants import ENTITY_TYPES
from draftjs_exporter.defaults import BLOCK_MAP
from draftjs_exporter.dom import DOM
from draftjs_exporter.html import HTML
import json

a = '''{
    "blocks": [
        {
            "key": "bh6r4",
            "text": "🙄😖",
            "type": "unstyled",
            "depth": 0,
            "inlineStyleRanges": [],
            "entityRanges": [
                {
                "offset": 0,
                "length": 1,
                "key": 7
                },
                {
                "offset": 1,
                "length": 1,
                "key": 8
                }
            ],
            "data": {}
        }
    ],
    "entityMap": {
        "7": {
            "type": "emoji",
            "mutability": "IMMUTABLE",
            "data": {
                "emojiUnicode": "🙄"
            }
        },
        "8": {
            "type": "emoji",
            "mutability": "IMMUTABLE",
            "data": {
                "emojiUnicode": "😖"
            }
        }
    }
}'''

def emoji(props):
    emoji_encode = []
    for c in props.get('emojiUnicode'):
        code = '%04x' % ord(c)
        if code != '200d':
            emoji_encode.append('%04x' % ord(c))

    return DOM.create_element('span', {
        'data-emoji': '-'.join(emoji_encode),
        'class': 'emoji',
    }, props['children'])

def entity_fallback(props):
    return DOM.create_element('span', {'class': 'missing-entity'},
                              props['children'])

def style_fallback(props):
    return props['children']

def block_fallback(props):
    return DOM.create_element('div', {}, props['children'])

DRAFTJS_EXPORTER_CONFIG = {
    'entity_decorators': {
        'emoji': emoji,
        ENTITY_TYPES.FALLBACK: entity_fallback,
    },
    'block_map': dict(BLOCK_MAP, **{
        BLOCK_TYPES.FALLBACK: block_fallback,
    })
}

exporter = HTML(DRAFTJS_EXPORTER_CONFIG)

if __name__ == '__main__':
    print(exporter.render(json.loads(a)))

Actual output:

<p><span class="emoji" data-emoji="1f644">🙄</span>😖<span class="emoji" data-emoji="1f616"></span></p>

Expected output:

<p><span class="emoji" data-emoji="1f644">🙄</span><span class="emoji" data-emoji="1f616">😖</span></p>
ericpai commented 6 years ago

@thibaudcolas Maybe the bug is caused by draftjs_exporter.html.build_command_groups?

thibaudcolas commented 6 years ago

Hey @ericpai, thanks for reporting this and taking the time to provide a self-contained example.

build_command_groups does sound like the most likely place for a bug like this. I'll investigate and hopefully get this fixed ASAP.

If you want to help out, a first step would be to add a (failing) test for this next to the one for adjacent inline styles, over at https://github.com/springload/draftjs_exporter/blob/cafdffcdeb35888882d98d6db1f0b99e417ebb3e/tests/test_exports.json#L118-L149.

ericpai commented 6 years ago

@thibaudcolas I have added test cases for this issue.

thibaudcolas commented 6 years ago

Thanks @ericpai, that helps. I've had a look, wasn't able to fully figure it out just yet. From what I can see the commands grouping is correct, I think the issue is that the entity state completely resets at the "stop" command of an entity – even if another entity also just had its "start" command. I'll investigate further.

Relevant code for reference:

https://github.com/springload/draftjs_exporter/blob/cafdffcdeb35888882d98d6db1f0b99e417ebb3e/draftjs_exporter/html.py#L68-L79

https://github.com/springload/draftjs_exporter/blob/cafdffcdeb35888882d98d6db1f0b99e417ebb3e/draftjs_exporter/entity_state.py#L45-L61

I think (but have yet to confirm) that the issue is the reset of the element_stack here https://github.com/springload/draftjs_exporter/blob/cafdffcdeb35888882d98d6db1f0b99e417ebb3e/draftjs_exporter/entity_state.py#L61. The sequence of apply of commands and rendering isn't correct in that case.

thibaudcolas commented 6 years ago

Yep, I've found the issue and a fix: when an entity is marked as complete and rendered, the entity state doesn't handle starting the rendering of another entity at the same time. All other tests pass so I think the fix is good, but I'll do a bit of refactoring to make sure the code stays understandable.

thibaudcolas commented 6 years ago

@ericpai v2.1.5 is now available on PyPI, with the fix! Thanks again for the help 🙂

https://pypi.org/project/draftjs_exporter/