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

Deleting highlighted text deletes old selection #849

Open
drewjenkins opened this issue Dec 7, 2016 · 4 comments
Open

Deleting highlighted text deletes old selection #849

drewjenkins opened this issue Dec 7, 2016 · 4 comments

Comments

@drewjenkins
Copy link

drewjenkins commented Dec 7, 2016

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

What is the current behavior?
Highlighting text then deleting it deletes text at the previous selection state instead of the current selection.

defect

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. You can use this jsfiddle to get started:

  1. Type out a sentence
    1a. Take note of the current cursor position
  2. Highlight some text in the middle of the sentence
  3. Press delete/backspace.
    Expected Results:
    Text under the highlighting is deleted
    Actual Results:
    Text at the previous selection is deleted

This issue is reproducible on facebook.com's "What's on your mind?" editor.

What is the expected behavior?
Highlighted text should be deleted

Which versions of Draft.js, and which browser / OS are affected by this issue? Did this work in previous versions of Draft.js?
Experiencing this on Mac OSX 10.11.6 Chrome Version 55.0.2883.75 beta (64-bit). Have customers who have reported seeing it on Windows 7 Browser Control (pretty much a hacky version of IE) as well.

Unsure if this worked in previous versions of draft. We are using draft-js 0.9.1.

@davidchang
Copy link
Contributor

@drewjenkins - thanks for opening this. this is crazy, definitely a bug.

are you able to see it on the editor at this page? https://facebook.github.io/draft-js/ i tried there as well as on the "What's on your mind?" editor on Facebook and wasn't able to repro the behavior (though my Mac OSX was 10.12 and Chrome version 54). do you have any browser extensions?

@drewjenkins
Copy link
Author

Hi @davidchang

I'm able to reproduce on that page as well. I can also try on some other browsers as well. I'll let you know what I see.

repro

@drewjenkins
Copy link
Author

Here are some preliminary testing results

Reproducible
Windows 10 - IE11 (Even Crazier issue, deletes all but the very first letter of whatever you typed, regardless of amount of text highlighted)
Windows 10 - WPF Web Browser Control (Same issue as IE11)
Mac OSX 10.11.6 - Firefox 49
Mac OSX 10.11.6 - Safari 10 (11602.1.50.0.10)
Mac OSX 10.11.6 - Chrome 56.0.2924.28 beta (64-bit)
Mac OSX 10.11.6 - Chrome 30 (Just because I happened to have it installed, but makes me think it is more tied to the OS than the browser for Mac)

Not Reproducible
Windows 10 - Chrome 55.0.2883.87
Windows 10 - Chromium hosted in Browser Control
Windows 10 - Firefox 48.0
Windows 10 - Microsoft Edge

@colinjeanne
Copy link
Contributor

The Windows 10 IE 11 and WPF Web Browser Control issues are due to #917.

flarnie added a commit to flarnie/draft-js that referenced this issue Jan 9, 2017
…archive#667"

This fixes some issues with the fix originally merged from PR facebookarchive#667[2]

Credit to @balpert and @srmcconomy for this fix. For now I'm getting
them back in sync.

Also thanks to @davidchang for pinging me about landing this,
@colinjeane for reporting this bug in facebookarchive#917 and submitting a nearly
identical fix: facebookarchive#919

@colinjeane since we already had this fixed internally I'm going to use
our approach to keep things in sync.

fixes facebookarchive#917, facebookarchive#849

[2]: facebookarchive#667

We also had to tweak this compared to the current internal version to
accomodate a recent change that makes 'editOnBeforeInput' and the other
handlers take 'editor' as an explicit argument instead of assumming
'this' will be the editor;
facebookarchive@e64c2c3

Note that we are updating the version of 'fbjs' required in order to get
access to the 'setImmediate' polyfill.
flarnie added a commit that referenced this issue Jan 12, 2017
)

This fixes some issues with the fix originally merged from PR #667[2]

Credit to @balpert and @srmcconomy for this fix. For now I'm getting
them back in sync.

Also thanks to @davidchang for pinging me about landing this,
@colinjeane for reporting this bug in #917 and submitting a nearly
identical fix: #919

@colinjeane since we already had this fixed internally I'm going to use
our approach to keep things in sync.

fixes #917, #849

[2]: #667

We also had to tweak this compared to the current internal version to
accomodate a recent change that makes 'editOnBeforeInput' and the other
handlers take 'editor' as an explicit argument instead of assumming
'this' will be the editor;
e64c2c3

Note that we are updating the version of 'fbjs' required in order to get
access to the 'setImmediate' polyfill.
max-winderbaum pushed a commit to textioHQ/draft-js that referenced this issue Jan 23, 2017
…archive#667" (facebookarchive#871)

This fixes some issues with the fix originally merged from PR facebookarchive#667[2]

Credit to @balpert and @srmcconomy for this fix. For now I'm getting
them back in sync.

Also thanks to @davidchang for pinging me about landing this,
@colinjeane for reporting this bug in facebookarchive#917 and submitting a nearly
identical fix: facebookarchive#919

@colinjeane since we already had this fixed internally I'm going to use
our approach to keep things in sync.

fixes facebookarchive#917, facebookarchive#849

[2]: facebookarchive#667

We also had to tweak this compared to the current internal version to
accomodate a recent change that makes 'editOnBeforeInput' and the other
handlers take 'editor' as an explicit argument instead of assumming
'this' will be the editor;
facebookarchive@e64c2c3

Note that we are updating the version of 'fbjs' required in order to get
access to the 'setImmediate' polyfill.
flarnie added a commit that referenced this issue Jan 27, 2017
)

This fixes some issues with the fix originally merged from PR #667[2]

Credit to @balpert and @srmcconomy for this fix. For now I'm getting
them back in sync.

Also thanks to @davidchang for pinging me about landing this,
@colinjeane for reporting this bug in #917 and submitting a nearly
identical fix: #919

@colinjeane since we already had this fixed internally I'm going to use
our approach to keep things in sync.

fixes #917, #849

[2]: #667

We also had to tweak this compared to the current internal version to
accomodate a recent change that makes 'editOnBeforeInput' and the other
handlers take 'editor' as an explicit argument instead of assumming
'this' will be the editor;
e64c2c3

Note that we are updating the version of 'fbjs' required in order to get
access to the 'setImmediate' polyfill.
ouchxp pushed a commit to ouchxp/draft-js that referenced this issue Apr 7, 2017
…archive#667" (facebookarchive#871)

This fixes some issues with the fix originally merged from PR facebookarchive#667[2]

Credit to @balpert and @srmcconomy for this fix. For now I'm getting
them back in sync.

Also thanks to @davidchang for pinging me about landing this,
@colinjeane for reporting this bug in facebookarchive#917 and submitting a nearly
identical fix: facebookarchive#919

@colinjeane since we already had this fixed internally I'm going to use
our approach to keep things in sync.

fixes facebookarchive#917, facebookarchive#849

[2]: facebookarchive#667

We also had to tweak this compared to the current internal version to
accomodate a recent change that makes 'editOnBeforeInput' and the other
handlers take 'editor' as an explicit argument instead of assumming
'this' will be the editor;
facebookarchive@e64c2c3

Note that we are updating the version of 'fbjs' required in order to get
access to the 'setImmediate' polyfill.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants