fixed a bug related to draft losing its selection #1571

pferdone · 2017-12-12T15:18:56Z

Summary

Sometimes rangeCount <= 0 and Draft tries to access an index that's out of range. See stackoverflow for a simplified example. Chrome throws the following exception:

react-dom.development.js:10994 Uncaught DOMException: Failed to execute 'getRangeAt' on 'Selection': 0 is not a valid index.
    at addFocusToSelection (http://localhost:3002/vendor.bundle.js:47834:27)
    at setDraftEditorSelection (http://localhost:3002/vendor.bundle.js:47728:5)
    at DraftEditorLeaf._setSelection (http://localhost:3002/vendor.bundle.js:45761:5)
    at DraftEditorLeaf.componentDidUpdate (http://localhost:3002/vendor.bundle.js:45771:10)
    at commitLifeCycles (http://localhost:3002/vendor.bundle.js:34699:24)
    at commitAllLifeCycles (http://localhost:3002/vendor.bundle.js:35867:9)
    at HTMLUnknownElement.callCallback (http://localhost:3002/vendor.bundle.js:26463:14)
    at Object.invokeGuardedCallbackDev (http://localhost:3002/vendor.bundle.js:26502:16)
    at invokeGuardedCallback (http://localhost:3002/vendor.bundle.js:26359:27)
    at commitRoot (http://localhost:3002/vendor.bundle.js:35971:9)

Fix

I added a rangeCount check.

TimSusa · 2017-12-12T16:45:15Z

Amazing! We would love to have this!

juliankrispel

Thanks so much for this @pferdone! Just left one comment about adding a test to protect against regressions. 👍

juliankrispel · 2017-12-13T05:21:01Z

src/component/selection/setDraftEditorSelection.js

    // after the range is detached.
-    var range = selection.getRangeAt(0);
-    range.setEnd(node, offset);
-    selection.addRange(range.cloneRange());


mind adding a failing test for the above and then adding your test which fixes it? That'd prevent a regression

juliankrispel · 2017-12-13T05:21:47Z

src/component/selection/setDraftEditorSelection.js

+    //
+    // Added rangeCount check
+    // https://stackoverflow.com/questions/22935320/uncaught-indexsizeerror-failed-to-execute-getrangeat-on-selection-0-is-not
+    if (selection.rangeCount > 0) {


I've run into this before, legit error! Thanks for the pr! I'll bring it up and hopefully we can merge this in soonish

amensouissi · 2018-01-15T16:02:54Z

Pleas merge....

nearwood · 2018-11-16T19:46:34Z

I still experience this with Draft-js v 0.10.5, with IE in this IE specific code in setDraftEditorSelection.js:

// IE doesn't support extend. This will mean no backward selection.
// Extract the existing selection range and add focus to it.
// Additionally, clone the selection range. IE11 throws an
// InvalidStateError when attempting to access selection properties
// after the range is detached.
var range = selection.getRangeAt(0); //<-- BOOM
range.setEnd(node, offset);
selection.addRange(range.cloneRange());

I have a workaround but I had to try and hide it in a larger commit it's so ugly.

Sigma-90 · 2019-07-08T16:02:10Z

Is there any particular reason why this really simple bugfix has not been merged yet after more than 1.5 years? I am experiencing the same problem nearwood described last november and it crashes the entire app in IE11 (can't ignore IE because of client requirements). The only way I could make it work is applying this fix manually and working with a fixed local dependency, which of course is super ugly and cuts the app off from any future updates to draft.js.

…selection when there is not one (#2271) Summary: **Summary** Various browsers will throw errors if `selection.getRangeAt(0)` is called when there is no range. To fix this, a check was added to make sure a range exists. This change was originally suggested [here](#1571) a few years ago. I opted for opening a new PR and adding tests here since that PR seemed stale. Selection and Range did not work in the test environment, so I had to stub them. I only added functionality for what was needed in the tests I added, but more can be added in the future very easily. Pull Request resolved: #2271 Differential Revision: D18807105 Pulled By: mrkev fbshipit-source-id: 0e3b833b8a3267b9a5f17b262b6a0442b6ae5e3d

fabiomcosta · 2019-12-30T22:29:37Z

ping?

arnaudambro · 2020-03-23T10:24:04Z

hello ? I also have the same error

…selection when there is not one (facebookarchive#2271) Summary: **Summary** Various browsers will throw errors if `selection.getRangeAt(0)` is called when there is no range. To fix this, a check was added to make sure a range exists. This change was originally suggested [here](facebookarchive#1571) a few years ago. I opted for opening a new PR and adding tests here since that PR seemed stale. Selection and Range did not work in the test environment, so I had to stub them. I only added functionality for what was needed in the tests I added, but more can be added in the future very easily. Pull Request resolved: facebookarchive#2271 Differential Revision: D18807105 Pulled By: mrkev fbshipit-source-id: 0e3b833b8a3267b9a5f17b262b6a0442b6ae5e3d

mrkev · 2020-03-25T22:34:37Z

This was already merged in #2271

…selection when there is not one (facebookarchive#2271) Summary: **Summary** Various browsers will throw errors if `selection.getRangeAt(0)` is called when there is no range. To fix this, a check was added to make sure a range exists. This change was originally suggested [here](facebookarchive#1571) a few years ago. I opted for opening a new PR and adding tests here since that PR seemed stale. Selection and Range did not work in the test environment, so I had to stub them. I only added functionality for what was needed in the tests I added, but more can be added in the future very easily. Pull Request resolved: facebookarchive#2271 Differential Revision: D18807105 Pulled By: mrkev fbshipit-source-id: 0e3b833b8a3267b9a5f17b262b6a0442b6ae5e3d

fixed a bug related to draft losing its selection

02668d6

facebook-github-bot added the CLA Signed label Dec 12, 2017

juliankrispel suggested changes Dec 13, 2017

View reviewed changes

zackham added a commit to zackham/draft-js that referenced this pull request Jun 13, 2019

added rangeCount check. see facebookarchive#1571

119dd58

zackham added a commit to zackham/draft-js that referenced this pull request Jun 13, 2019

added rangeCount check. see facebookarchive#1571

2ada60d

zackham added a commit to zackham/draft-js that referenced this pull request Aug 8, 2019

added rangeCount check. see facebookarchive#1571

761186a

LaurenWyatt610 mentioned this pull request Dec 3, 2019

Added tests and fixed IE IndexSizeError trying to get a range from a selection when there is not one #2271

Closed

mrkev closed this Mar 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fixed a bug related to draft losing its selection #1571

fixed a bug related to draft losing its selection #1571

Uh oh!

pferdone commented Dec 12, 2017

Uh oh!

TimSusa commented Dec 12, 2017

Uh oh!

juliankrispel left a comment

Uh oh!

juliankrispel Dec 13, 2017

Uh oh!

juliankrispel Dec 13, 2017

Uh oh!

amensouissi commented Jan 15, 2018

Uh oh!

nearwood commented Nov 16, 2018

Uh oh!

Sigma-90 commented Jul 8, 2019

Uh oh!

fabiomcosta commented Dec 30, 2019

Uh oh!

arnaudambro commented Mar 23, 2020

Uh oh!

mrkev commented Mar 25, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

fixed a bug related to draft losing its selection #1571

fixed a bug related to draft losing its selection #1571

Uh oh!

Conversation

pferdone commented Dec 12, 2017

Uh oh!

TimSusa commented Dec 12, 2017

Uh oh!

juliankrispel left a comment

Choose a reason for hiding this comment

Uh oh!

juliankrispel Dec 13, 2017

Choose a reason for hiding this comment

Uh oh!

juliankrispel Dec 13, 2017

Choose a reason for hiding this comment

Uh oh!

amensouissi commented Jan 15, 2018

Uh oh!

nearwood commented Nov 16, 2018

Uh oh!

Sigma-90 commented Jul 8, 2019

Uh oh!

fabiomcosta commented Dec 30, 2019

Uh oh!

arnaudambro commented Mar 23, 2020

Uh oh!

mrkev commented Mar 25, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants