Entity editing broke with recent DraftEditorCompositionHandler updates

[sarmstrong]

Recent updates to DraftEditorCompositionHandler seems to have broken how entities are updated.

Take this example from https://github.com/facebook/draft-js/blob/master/examples/draft-0-10-0/entity/entity.html

This screenshot demonstrates current behavior. The entity is not correctly updated when using 2-Set Korean and throws an error.

Here is a screenshot of the behavior before the update

Let me know if you need anything more. For recreation, I use the 2-Set Korean on Mac for testing. Typing "tkfk" + "enter" produces the content you see in the screenshot.

Recent updates to DraftEditorCompositionHandler do seem to solve a lot of issues, so hopefully this isn't discouraging.

• @fabiomcosta

[fabiomcosta]

This is a perfect bug report, thank you Steve! I’ll try to check what’s happening over this week.

On Tue, Jun 11, 2019 at 3:22 PM Steve Armstrong notifications@github.com wrote:

Recent updates to DraftEditorCompositionHandler https://github.com/facebook/draft-js/blob/master/src/component/handlers/composition/DraftEditorCompositionHandler.js seems to have broken how entities are updated.

Take this example from https://github.com/facebook/draft-js/blob/master/examples/draft-0-10-0/entity/entity.html

This screenshot demonstrates current behavior. The entity is not correctly updated when using 2-Set Korean and throws an error. [image: df498672882d65f74c595a85325085e1] https://user-images.githubusercontent.com/928849/59310501-8e1ef800-8c74-11e9-9ec0-8e9ee5790731.gif

Here is a screenshot of the behavior before the update [image: d752df6d747f1b7a8970726f720823f1] https://user-images.githubusercontent.com/928849/59310512-9b3be700-8c74-11e9-907b-78cb8a5f2add.gif

Let me know if you need anything more. For recreation, I use the 2-Set Korean on Mac for testing. Typing "tkfk" + "enter" produces the content you see in the screenshot.

Recent updates to DraftEditorCompositionHandler do seem to solve a lot of issues, so hopefully this isn't discouraging.

• @fabiomcosta https://github.com/fabiomcosta

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/facebook/draft-js/issues/2104?email_source=notifications&email_token=AAAG64UMUIYAWXZDNSHXUVDP2AQSZA5CNFSM4HXD2HUKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GY5VQ6Q, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAG64TGGF3LHIBH6HTJE6LP2AQSZANCNFSM4HXD2HUA .

--

Fabio Miranda Costa twitter: @fabiomiranda github: fabiomcosta

[fabiomcosta]

I see the issue, but still trying to see what's the right way to fix it. When getDraftEditorSelection is called at https://github.com/facebook/draft-js/blob/master/src/component/handlers/composition/DraftEditorCompositionHandler.js#L220 editorState is already properly updated and without the removed immutable "Superman" entity, but the DOM is still on the old state, so when global.getSelection() is called, the immutable entity is detected to be the leaf node of the selection, but since it doesn't exist on editorState the error ends up happening.

I'll think about this a bit more to see if I can come up with a clean fix.

[claudiopro]

Thanks for reporting @sarmstrong and taking a look into it and proposing a fix @fabiomcosta 💯

Is there any way we can unit test this behavior so we don't regress in the future? Similarly, are we sure this is not causing regression in unrelated input flows?

[fabiomcosta]

Good call Claudio, I’ll see if I can come up with more testing that catches some of the known issues

On Wed, Jun 12, 2019 at 11:43 PM Claudio Procida notifications@github.com wrote:

Thanks for reporting @sarmstrong https://github.com/sarmstrong and taking a look into it and proposing a fix @fabiomcosta https://github.com/fabiomcosta 💯

Is there any way we can unit test this behavior so we don't regress in the future? Similarly, are we sure this is not causing regression in unrelated input flows?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/facebook/draft-js/issues/2104?email_source=notifications&email_token=AAAG64WF3RMTKSRREQ55STDP2HT75A5CNFSM4HXD2HUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXSV3DY#issuecomment-501570959, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAG64VBGO52EEIUPWJBKBDP2HT75ANCNFSM4HXD2HUA .

--

Fabio Miranda Costa twitter: @fabiomiranda github: fabiomcosta