-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Added tests and fixed IE IndexSizeError trying to get a range from a selection when there is not one #2271
Conversation
a4ce2f8
to
45b0798
Compare
This is awesome! Thanks for adding the tests too. I'll try to get this merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrkev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Nice! |
@mrkev Is there anything else I need to do (resolving conflicts and making changes based on whatever the internal test failures/warnings are)? Or is that handed by Facebook now since this PR has been "imported" |
Hey @lrnwytt, holiday season has lots of things fighting for my attention, thanks for bringing this up again, I want to get this in 👍 Facebook web exists as a monorepo. Had a quick look at the test failure— it seems that the way this repo is set up to sync with our www monorepo means that adding files on Github doesn't add them to our dependency resolution system, so the tests can't actually find Let's just embed the The lint failure is actually a warning because the test is not |
45b0798
to
daffa66
Compare
…selection when there is not one
daffa66
to
fc5b32a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrkev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Sweet thanks! |
Thanks for working through this with me! We just got this logged as a bug and it's so great to see how easy the outsider PR process is. |
@lrnwytt thanks, this was a very well-done PR which made it easy. Hope you have a great holiday too! (: |
…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
…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
…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](facebookarchive/draft-js#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/draft-js#2271 Differential Revision: D18807105 Pulled By: mrkev fbshipit-source-id: 0e3b833b8a3267b9a5f17b262b6a0442b6ae5e3d
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 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.