draft-js icon indicating copy to clipboard operation
draft-js copied to clipboard

Lost position of the cursor when editing

Open cestrensem opened this issue 8 years ago • 49 comments
trafficstars

Do you want to request a feature or report a bug? Bug

What is the current behavior? User already has some text in the text area. He puts cursor with his mouse somewhere and quickly types something, then again quickly puts cursor to other place in the text area and again quickly types something. After these several "puts and types" the place where the cursor is and the place where the typed text appears are not in sync anymore. The cursor is in one place, the text appears in another place of the text area.

**If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. https://youtu.be/RZUzLyvLgnY

What is the expected behavior? Text appears on the same position where the cursor is.

Which versions of Draft.js, and which browser / OS are affected by this issue? Did this work in previous versions of Draft.js? Draft js 0.10.0 & 0.9.1.

Browsers:

  • Chrome 55.0.2883.95 (64-bit) Mac OS Sierra 11.02.12
  • Safari 10.0.2 Mac OS Sierra 11.02.12
  • Firefox 49.0.0 Mac OS Sierra 11.02.12
  • IE 11 WIndows 10
  • EDGE 13 Windows 10

cestrensem avatar Jan 31 '17 18:01 cestrensem

Hi @cestrensem - thanks for reporting this and making the recording of the problem! This definitely does not look good, and I will look into it as soon as I get a chance. I'd also be happy to review any PR that has a solution.

It looks like that is a large area with many blocks, and since we have seen performance issues with rendering a large number of text blocks I wonder if that is contributing to this problem also.

flarnie avatar Feb 01 '17 17:02 flarnie

Having a large number of blocks seems to make this easier to repro but I find it pretty easy to repro with just two blocks with a few words each (and a decorator that does a small amount of work). I'm trying to investigate this and it seems - so far - that the issue is that a call to getUpdatedSelectionState appears to be returning the wrong selection when typing quickly causing Draft and the browser to disagree where the caret actually is.

This repros only when native insertion is allowed, which makes sense. The issue is fundamentally that Draft and the browser are disagreeing about where selection is. The browser inserts text in one location and Draft inserts it in another.

This is not, however, the result of the setImmediate call in editOnBeforeInput. We have a local branch which removes this call due to another issue which we have yet to file on Facebook's Draft repo and this issue still occurs in that branch.

colinjeanne avatar Feb 01 '17 22:02 colinjeanne

feb-01-2017 14-26-37

The bug repros at frame 49 of this GIF. The Before input active logs on this line, the DOM selection is being logged on this one and selection unchanged means that this line was hit.

colinjeanne avatar Feb 01 '17 22:02 colinjeanne

Here's what I believe is happening:

getUpdatedSelectionState is claiming that that the old and new selections are equal because it's being given the nodes for the previously selected block. These nodes are ultimately coming from the DOM selection in getDraftEditorSelection so it seems that the DOM's selection hasn't been updated yet. The third z is added to the first block by the browser during its native insertion and the third z is added by Draft in editOnBeforeInput where it thinks the browser is adding the z.

I don't yet know why the data from the block that has the browser's selection is copied over the corresponding data in the second block but that is a common occurrence I see in similar issues.

colinjeanne avatar Feb 02 '17 05:02 colinjeanne

@colinjeanne I have a simlar problem like this, so how can i slove it? thanks a lot

dreamcog avatar Feb 11 '17 05:02 dreamcog

@dreamcog I don't yet know. I haven't had a chance to dig further into this since I wrote the above post and am hoping that someone else is able to use that information to come up with a fix since I'm unlikely to be able to get to it soon.

colinjeanne avatar Feb 13 '17 18:02 colinjeanne

I have almost same problem. When type fast my text is broken and go to another line. As i see DOM is breaking and text appear in parent DOM element. I have same problem on FF in 0.9.0 but no in 0.10.0...in this version only IE 11 is buggy. Does anyone have a clue what could it be ?

Milos5611 avatar Feb 20 '17 10:02 Milos5611

Looking at the playbacks I notice that the bug repros after I click in a block in the frame immediately following my log in editOnBeforeInput. At this point Draft appears to be in a bad state and so the next character press causes it to improperly copy text from a different block while rectifying its state with what's in the DOM. @max-winderbaum confirmed separately that adding a call to editOnSelect in editOnSelect to handle a change in selection between the onBeforeInput and onInput events does fix this bug at the expense of another bug we are currently attempting to fix.

Further, this bug can only repro when native insertion is enabled.

For those playing at home, onBeforeInput is not a real browser event but is one that React is emulating. This appears to leave open two windows for the selection to change

  1. After onBeforeInput is handled but before the native insertion is processed (not shown in this bug)
  2. After the native insertion but before editOnInput (this bug)

There is further work to come up with the right fix but this seems to be a good working model for what's happening and implies avenues to create a proper fix.

colinjeanne avatar Feb 21 '17 20:02 colinjeanne

@flarnie: We have a fix for this issue - https://github.com/textioHQ/draft-js/pull/39

The diff here may seem a bit unfamiliar because it builds off of https://github.com/textioHQ/draft-js/pull/24 which is yet another variant of #667. There is a bug that we haven't yet filed in this repository that affects Draft in general and which motivated this change (Edit: now filed as #1018). That's unrelated to this bug, though.

The idea behind this change is that since selection may change between editOnBeforeInput and editOnInput we need to first recover the correct selection upon entry into editOnInput. If the selection changed then we also know that the block we had preemptively updated in editOnBeforeInput is not the block that was updated by the browser and so we replace it with that block's state from before editOnBeforeInput and let editOnInput fix up the block that actually was updated.

colinjeanne avatar Feb 21 '17 21:02 colinjeanne

@flarnie https://github.com/facebook/draft-js/pull/1042 should fix this issue

max-winderbaum avatar Feb 28 '17 23:02 max-winderbaum

Any updates on this issue. I get the same problem, but I can only edit/add text to the end of lines. If it is in the middle of the line, my cursor will select the text at that location. I cannot add spaces in the middle of the lines either.

mcnewbk avatar May 05 '17 20:05 mcnewbk

Hey guys, any updates???

cestrensem avatar Aug 23 '17 10:08 cestrensem

+1 would – kindly and politely – like an update :)

blakeperdue avatar Sep 11 '17 13:09 blakeperdue

any solution @max-winderbaum? In last release this error is present.

marlonmleite avatar Sep 27 '17 02:09 marlonmleite

Video bug: https://www.youtube.com/watch?v=EV-U8CS2qdk

This error only occurs when I initialize the state from my store with this:

componentWillReceiveProps(props) {
    const content = stateFromHTML(props.value || '')

    this.setState({
      editorState: EditorState.createWithContent(content)
    })
  }

marlonmleite avatar Sep 27 '17 02:09 marlonmleite

@marlonmleite: The original solution we had for this issue will need to be updated to account for changes in this area. We have not had time to integrate the latest Draft updates into our fork.

colinjeanne avatar Sep 27 '17 04:09 colinjeanne

@colinjeanne you have guidance or examples for me to do that workaround you said and solve my problem in this moment?

marlonmleite avatar Sep 27 '17 12:09 marlonmleite

I fix this bug in my project, with:

  componentWillMount() {
    const { value } = this.props

    this.state = {
      editorState: this.getCreatedEditorState(value)
    }
  }

  componentWillReceiveProps(nextProps) {
    const { value } = nextProps

    if (!this.props.value && value) {
      this.setState({ editorState: this.getCreatedEditorState(value) })
    }
  }

  getCreatedEditorState(value) {
    const contentState = stateFromHTML(value || '')

    return EditorState.createWithContent(contentState)
  }

marlonmleite avatar Sep 27 '17 13:09 marlonmleite

Hello, @cestrensem @flarnie @colinjeanne @max-winderbaum @mcnewbk, is there any solution? anybody fixed this? @marlonmleite where can i get stateFromHTML function?

againksy avatar Nov 01 '17 19:11 againksy

@cestrensem @flarnie @colinjeanne @max-winderbaum @mcnewbk, @marlonmleite Dears, the cursor jumps when beginning to enter text. Carret jumps before first entered letter. Also if we press backspace - it jumps to beginning of line , so what is the solution, maybe anyone knows?

againksy avatar Nov 01 '17 19:11 againksy

@againksy the imports are:

import { stateFromHTML } from 'draft-js-import-html'
import { stateToHTML } from 'draft-js-export-html'

I use stateToHTML in:

  onChange(editorState) {
    const content = editorState.getCurrentContent()
    const text = content.hasText() ? stateToHTML(content) : ''

    this.setEditorState(editorState, text)
  }

Seems to be the best solution for me.

marlonmleite avatar Nov 01 '17 20:11 marlonmleite

@marlonmleite what if i have not html in content? I have an draft object with entities and blocks ...

againksy avatar Nov 02 '17 07:11 againksy

I have a few moments to look at this again. It seems like it is even easier to hit this bug in the recent versions of Draft. This GIF is in Chrome on draftjs.org and the repo is simple: have the caret on the top line and press a key while simultaneously clicking on the other line. That indicates to me that the original fix we had created likely still applies (but I will test more completely once I bring our fork in sync with what's in the main repository).

nov-06-2017 08-21-57

colinjeanne avatar Nov 06 '17 16:11 colinjeanne

Was having the same issue, but in my case this ended up being related the https://www.draft-js-plugins.com/plugin/focus and how I was setting focus.

The focus plugin wants you to wrap the editor with div with onClick that sets focus. In my case I just commented out the focus onClick and voila the bug went away!

I don't quite understand the depth of the need for the onClick, with the focus plugin, but can't find any negative effects yet.

Hope this helps some others.

myyellowshoe avatar Nov 22 '17 15:11 myyellowshoe

Same issue as #424 and #910 it seems.

thibaudcolas avatar Jan 19 '18 14:01 thibaudcolas

Any updates on this issue? @cestrensem @flarnie @colinjeanne @max-winderbaum @mcnewbk, @marlonmleite ? Did the changes from the textio fork land into the Draft repo?

roundrobin avatar Mar 08 '18 17:03 roundrobin

I have a PR into the Facebook's repo, #1609, but it has not yet been checked in.

colinjeanne avatar Mar 08 '18 21:03 colinjeanne

@flarnie is there a plan to merge this PR anytime soon?

roundrobin avatar Mar 13 '18 10:03 roundrobin

@flarnie any update on this? Fixing this issue would really improve the user experience.

roundrobin avatar Apr 10 '18 12:04 roundrobin

@colinjeanne Do you know if it's possible to use this PR in my app so that it works with the Draft.js plugins?

I tried referencing this PR in my package.json but then the Draft.js plugins were complaining.

roundrobin avatar Apr 10 '18 14:04 roundrobin

@roundrobin if you mean https://www.draft-js-plugins.com then yes, this PR should work with at least the base editor. My fork of Draft with this change is sync'd to somewhere between the 10.1 and 10.2 Draft releases but I do use draft-js-plugins for extended editor component. I don't use any of the other plugins.

I am curious, though, what the complaints are?

colinjeanne avatar Apr 10 '18 15:04 colinjeanne

I'm trying to reproduce this again, but know I'm struggling to install this PR with NPM. Do you know the command to add this specific PR to my node_modules?

roundrobin avatar Apr 10 '18 15:04 roundrobin

You likely can't install this PR with npm. Instead you'd have to build Draft locally and sync to this change or otherwise merge this PR.

colinjeanne avatar Apr 10 '18 15:04 colinjeanne

I do not but I don't think it's related to this PR.

colinjeanne avatar Apr 10 '18 16:04 colinjeanne

You're right, this was unrelated to this PR, so I removed the comment because it didn't help the discussion.

For others: at the end I was able to include your PR in my workflow (npm + webpack), by including the dist folder in my forked version that includes the PR 1609, which webpack needs as as an entry point.

Thanks @colinjeanne for your contribution.

roundrobin avatar Apr 10 '18 17:04 roundrobin

  componentWillMount() {
    const { value } = this.props

    this.state = {
      editorState: this.getCreatedEditorState(value)
    }
  }

  componentWillReceiveProps(nextProps) {
    const { value } = nextProps

    if (!this.props.value && value) {
      this.setState({ editorState: this.getCreatedEditorState(value) })
    }
  }

  getCreatedEditorState(value) {
    const contentState = stateFromHTML(value || '')

    return EditorState.createWithContent(contentState)
  }

Worked for me as well (Y)

Tarun1987 avatar Dec 17 '18 13:12 Tarun1987

componentWillReceiveProps(nextProps) { const { value } = nextProps if (!this.props.value && value) { this.setState({ editorState: this.getCreatedEditorState(value) }) } }

I'm also having this issue and no way to resolve, can you elaborate further on your solution?

dasm30 avatar Jan 15 '19 20:01 dasm30

  const newState = EditorState.createEmpty()
        this.setState({
          editorState: EditorState.moveFocusToEnd(newState)
        })

This worked for me

sandeepreddy19 avatar Jan 24 '19 23:01 sandeepreddy19

It appears as though the PRs from @max-winderbaum and the adjustments that @colinjeanne have made have gone unnoticed.

I used the fork that the Textio team created and can also confirm that the bug is no longer present in their build.

@claudiopro @fabiomcosta @gkz @dsainati1 seem to be the recent contributors nowadays on this repo: Sorry to bug you all but is there anything we can do to speed up this long open PR?

I can help code review or perhaps there is some business reason why your team isn't accepting the PR.

I would just like some idea of why. Otherwise it seems like the Textio fork has some bugs fixed and those entering this thread in 2019 can look there as an option (the code samples above are not quite what is desired for me).

camsjams avatar Mar 26 '19 02:03 camsjams

@camsjams, the PR hasn't gone unnoticed. The issue is that our fork is based on a very old version of Draft, approximately 0.10.2 if I recall correctly. There has been a lot of code change in this area since then and when I tried to update the PR to 0.10.5 I could no longer convince myself that the original solution was doing the thing I believed it should be doing. I don't believe it's insurmountable but it just takes time to do the investigation and I haven't had time to do it.

Internally to Textio we are unlikely to invest time here because our needs have changed significantly as well and we may not be able to continue to use Draft in the same way we are today.

colinjeanne avatar Mar 26 '19 03:03 colinjeanne

@colinjeanne Thank you for the response, this is good closure on this thread :+1: .

I understand that its a reasonably large change, and I appreciate the candor regarding the future of this library in your team's stack.

I may do some exploratory work here on the source code to see if there is anything I can tweak in the newer code.

camsjams avatar Mar 26 '19 17:03 camsjams

If you'd like to take a look, the PR I was working on was https://github.com/facebook/draft-js/pull/1609. The issue that I couldn't resolve is primarily restricted to editOnBeforeInput.js: the trick is to allow mustPreventNative to be false in as many cases as possible to increase performance but allowing native insertion is what opens up the race condition that causes this issue. I was unable to determine if my change was allowing native insertion in the correct situations.

colinjeanne avatar Mar 27 '19 04:03 colinjeanne

As far as I can tell, this issue is caused by the editor state selection having hasFocus: false when the editor actually has focus. Normally, the DraftEditorLeaf maintains cursor position, after text updates, by moving the cursor to roughly where it was before the update. However, it only does this if it thinks it is rendering a block that has focus. If it doesn't think it has focus, then it will just update the rendered text which causes the cursor to jump back to the start.

The cause of this, in any particular application, can vary. It can occur because there can be an async step between the editor onChange and updating the editorState prop for the editor. This can cause a delay between when the editor actually has focus and when it thinks it has focus. It can also occur because a block has focus and the editor state selections is manually updated but drops hasFocus: true.

I don't think this a bug per se, but more of a poor tolerance for bad input.

cdow avatar Oct 28 '19 22:10 cdow

There any news at this point?

Struggling my head out in here 😆

operfildoluiz avatar May 31 '20 06:05 operfildoluiz

I agree with @cdow's explanation. Would be great to improve this.

wdfinch avatar Oct 15 '20 00:10 wdfinch

The other solutions were causing it to always have focus at the end. If the user changed the cursor to the middle it would still jump to the end. This is the solution I came up with:

  const handleChange = editorState => {
    // For some reason after typing the first character Draft.js resets the
    // focus to the start.  We need to work around this.
    const newState = convertToText(editorState).length === 1
      ? EditorState.moveFocusToEnd(editorState)
      : editorState
    setEditorState(newState)
  }

brennancheung avatar Feb 24 '21 19:02 brennancheung

@flarnie Are you guys working on the fix?

celestemartins avatar Apr 26 '21 17:04 celestemartins

You have to use class component to have better control over component updates. You need to maintain the content HTML as a string in the state to compare new props and the current state easily.

class TextEditor2 extends React.Component {
    constructor(props) {
        super(props);
        this.state = {
            editorState: getEditorState(props.content),
            content: props.content
        }
    }
    handleEditorStateChange = (editorState) => {
        const content = draftToHtml(convertToRaw(editorState.getCurrentContent()));
        this.setState({editorState, content})
        this.props.onChange(content);
    }

    componentDidUpdate(prevProps) {
        if (prevProps.content !== this.props.content && this.props.content !== this.state.content) {
            this.setState({
                editorState: getEditorState(this.props.contnet),
                content: this.props.content
            })
        }
    }

    render() {
        const {editorState,} = this.state;
        const {placeholder, readOnly, classes, className} = this.props;
        return (<Editor
            editorState={editorState}
            wrapperClassName={clsx(classes.wrapper, className)}
            toolbarClassName={classes.toolbar}
            editorClassName={classes.editor}
            readOnly={readOnly}
            stripPastedStyles={true}
            placeholder={placeholder}
            onEditorStateChange={this.handleEditorStateChange}
            toolbarOnFocus
            toolbar={{
                options: ['fontFamily', 'fontSize', 'inline', 'colorPicker', 'link', 'list', 'textAlign', 'remove'],
                inline: {
                    options: ['bold', 'italic', 'underline', 'strikethrough'],
                },
                colorPicker: {
                    className: undefined,
                    component: undefined,
                    popupClassName: undefined,
                    colors: Colors,
                },
            }}
        />)
    }
}
export default withStyles(styles)(TextEditor2);

SubhasisDebsharma avatar Feb 05 '22 05:02 SubhasisDebsharma