This repository has been archived by the owner on Feb 6, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When `_pendingStateFromBeforeInput` was added it defered some work in updating the editor's state until the next `input` event is received. Unfortunately, IE 11 doesn't support this event on contenteditable elements and React does not handle this inconsistency and so the defered work was never processed. This change kicks off timer to fire as soon as we hand control back to the browser and so simulate the event occuring on IE.
flarnie
added a commit
to flarnie/draft-js
that referenced
this pull request
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 pull request
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 pull request
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 pull request
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 pull request
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.
midas19910709
added a commit
to midas19910709/draft-js
that referenced
this pull request
Mar 30, 2022
…871) 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: facebookarchive/draft-js#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]: facebookarchive/draft-js#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/draft-js@e64c2c3 Note that we are updating the version of 'fbjs' required in order to get access to the 'setImmediate' polyfill.
alicayan008
pushed a commit
to alicayan008/draft-js
that referenced
this pull request
Jul 4, 2023
…871) 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: facebookarchive/draft-js#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]: facebookarchive/draft-js#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/draft-js@e64c2c3 Note that we are updating the version of 'fbjs' required in order to get access to the 'setImmediate' polyfill.
aforismesen
added a commit
to aforismesen/draft-js
that referenced
this pull request
Jul 12, 2024
…871) 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: facebookarchive/draft-js#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]: facebookarchive/draft-js#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/draft-js@e18c8dc 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.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This fixes #917.
When
_pendingStateFromBeforeInput
was added it defered some work in updating the editor's state until the nextinput
event is received. Unfortunately, IE 11 doesn't support this event on contenteditable elements and React does not handle this inconsistency and so the defered work was never processed.This change kicks off timer to fire as soon as we hand control back to the browser and so simulate the event occuring on IE.
Test Plan
The automated tests currently don't handle this so I performed quite a bit of manual testing. The change is restricted to IE (and not Edge which correctly supports the event). I was particularly concerned that
setTimeout
wouldn't be a close enough approximation to the actual event after watching the code flow in several browsers as well as in IE with this fix it appears to me that this executes the same code in the same order across these browsers.