Skip to content
This repository has been archived by the owner on Feb 6, 2023. It is now read-only.

Lost position of the cursor when editing #989

Open
cestrensem opened this issue Jan 31, 2017 · 49 comments
Open

Lost position of the cursor when editing #989

cestrensem opened this issue Jan 31, 2017 · 49 comments

Comments

@cestrensem
Copy link

cestrensem commented Jan 31, 2017

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
@flarnie
Copy link
Contributor

flarnie commented Feb 1, 2017

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.

@colinjeanne
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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.

@dreamcog
Copy link

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

@colinjeanne
Copy link
Contributor

@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.

@Milos5611
Copy link

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 ?

@colinjeanne
Copy link
Contributor

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
Copy link
Contributor

colinjeanne commented Feb 21, 2017

@flarnie: We have a fix for this issue - textioHQ#39

The diff here may seem a bit unfamiliar because it builds off of textioHQ#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.

@max-winderbaum
Copy link

@flarnie #1042 should fix this issue

@mcnewbk
Copy link

mcnewbk commented May 5, 2017

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.

@cestrensem
Copy link
Author

Hey guys, any updates???

@blakeperdue
Copy link

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

@marlonmleite
Copy link

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

@marlonmleite
Copy link

marlonmleite commented Sep 27, 2017

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)
    })
  }

@colinjeanne
Copy link
Contributor

@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.

@marlonmleite
Copy link

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

@marlonmleite
Copy link

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)
  }

@againksy
Copy link

againksy commented Nov 1, 2017

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

@againksy
Copy link

againksy commented Nov 1, 2017

@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?

@colinjeanne
Copy link
Contributor

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

@roundrobin
Copy link

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

@roundrobin
Copy link

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

@roundrobin
Copy link

roundrobin commented Apr 10, 2018

@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.

@colinjeanne
Copy link
Contributor

@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?

@roundrobin
Copy link

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?

@colinjeanne
Copy link
Contributor

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
Copy link
Contributor

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

@roundrobin
Copy link

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.

@Tarun1987
Copy link

Tarun1987 commented Dec 17, 2018

  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)

@dasm30
Copy link

dasm30 commented Jan 15, 2019

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?

@sandeepreddy19
Copy link

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

This worked for me

@camsjams
Copy link

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).

@colinjeanne
Copy link
Contributor

@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.

@camsjams
Copy link

camsjams commented Mar 26, 2019

@colinjeanne Thank you for the response, this is good closure on this thread 👍 .

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.

@colinjeanne
Copy link
Contributor

If you'd like to take a look, the PR I was working on was #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.

@cdow
Copy link

cdow commented Oct 28, 2019

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.

@operfildoluiz
Copy link

There any news at this point?

Struggling my head out in here 😆

@wdfinch
Copy link

wdfinch commented Oct 15, 2020

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

@brennancheung
Copy link

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)
  }

@celestemartins
Copy link

celestemartins commented Apr 26, 2021

@flarnie Are you guys working on the fix?

ghost pushed a commit to Researchify/Researchify that referenced this issue Oct 11, 2021
Using raw content to transfer and passing the data, bugs happen: lost cursor position when editing. An issue with draftjs itself, need to work around with, see: facebookarchive/draft-js#989
@SubhasisDebsharma
Copy link

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);

@mitevdimitar
Copy link

mitevdimitar commented Oct 12, 2022

This little trick solved the issue with the duplicated rows for me. It seemed that the unwanted change of the state comes with lastChangeType called "apply-entity". I couldn't reproduce other cases where this change type occurred, so I isolated it by not updating the state in those cases:

onChange = (newState) => {
    const lastChangeType =
      newState && newState._immutable && newState._immutable.lastChangeType;
    if (lastChangeType === "apply-entity") return;
    this.setState({
      editorState: newState
    });
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet