This repository was archived by the owner on Feb 6, 2023. It is now read-only.
Modified composition mode event handlers to provide better support fo…#1152
Open
njjewers wants to merge 5 commits intofacebookarchive:mainfrom
Open
Modified composition mode event handlers to provide better support fo…#1152njjewers wants to merge 5 commits intofacebookarchive:mainfrom
njjewers wants to merge 5 commits intofacebookarchive:mainfrom
Conversation
…r mobile browsers
Author
|
Specifically, these changes should fix the issues described in #1107, #1079, and possibly #1128, as all of these are likely issues stemming from the differing (and arguably, non-spec compliant) sets of events generated by various combinations of OS, browser, and IME. The included test cases simulate a few different input combinations I've noticed during testing, as well as cases listed in the comments of draftEditorCompositionHandler. |
|
@flarnie Review this pls |
octatone
added a commit
to octatone/draft-js
that referenced
this pull request
Aug 9, 2017
This continues from facebookarchive#1152 and facebookarchive#1285 making them work together for broad support for polatform inconsistencies.
octatone
added a commit
to octatone/draft-js
that referenced
this pull request
Nov 21, 2017
This continues from facebookarchive#1152 and facebookarchive#1285 making them work together for broad support for polatform inconsistencies.
aminland
added a commit
to aminland/draft-js
that referenced
this pull request
Mar 13, 2018
* Modified compositoin mode event handlers to provide better support for mobile browsers * Added test cases for DraftEditorCompositionHandler * Linter compliance * Added support for `compositionupdate` event. * Made the linter happy! * Normalize beforeInput and compositionUpdate data This continues from facebookarchive#1152 and facebookarchive#1285 making them work together for broad support for polatform inconsistencies. * Put new composition fixes behind a feature flag * lint * correct resets * Fix Synthetic Event flow types * Convert DraftEditorCompositionHandler tests to root tests + snapshots * Remove unnecessary DraftFeatureFlags
NeXidan
pushed a commit
to NeXidan/draft-js
that referenced
this pull request
Sep 4, 2018
…r mobile browsers Added test cases for DraftEditorCompositionHandler Linter compliance Added support for `compositionupdate` event. Made the linter happy! Normalize beforeInput and compositionUpdate data This continues from facebookarchive#1152 and facebookarchive#1285 making them work together for broad support for polatform inconsistencies. Put new composition fixes behind a feature flag lint correct resets Fix Synthetic Event flow types Convert DraftEditorCompositionHandler tests to root tests + snapshots
NeXidan
pushed a commit
to NeXidan/draft-js
that referenced
this pull request
Sep 24, 2018
…r mobile browsers Added test cases for DraftEditorCompositionHandler Linter compliance Added support for `compositionupdate` event. Made the linter happy! Normalize beforeInput and compositionUpdate data This continues from facebookarchive#1152 and facebookarchive#1285 making them work together for broad support for polatform inconsistencies. Put new composition fixes behind a feature flag lint correct resets Fix Synthetic Event flow types Convert DraftEditorCompositionHandler tests to root tests + snapshots
NeXidan
pushed a commit
to NeXidan/draft-js
that referenced
this pull request
Sep 24, 2018
…r mobile browsers Added test cases for DraftEditorCompositionHandler Linter compliance Added support for `compositionupdate` event. Made the linter happy! Normalize beforeInput and compositionUpdate data This continues from facebookarchive#1152 and facebookarchive#1285 making them work together for broad support for polatform inconsistencies. Put new composition fixes behind a feature flag lint correct resets Fix Synthetic Event flow types Convert DraftEditorCompositionHandler tests to root tests + snapshots
facebook-github-bot
left a comment
There was a problem hiding this comment.
@nmn has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
pushed a commit
that referenced
this pull request
Mar 5, 2020
Summary: Using a Pull Request from github that was made to fix this very bug, I was able to patch the code and fix the problem for us in the meantime. Since the open-source and internal codebases are totally out of sync, it's not easy to merge the original PR right now. #1152 Reviewed By: claudiopro Differential Revision: D20016281 fbshipit-source-id: aeefd5c3f17c51c62dd5d7c225cf62d0cd2db26f
Contributor
|
@njjewers I tried to pull in your PR. But since it's been sitting here for so long it no longer works consistently. Any chance you could rebase it? |
mmissey
pushed a commit
to mmissey/draft-js
that referenced
this pull request
Mar 24, 2020
Summary: Using a Pull Request from github that was made to fix this very bug, I was able to patch the code and fix the problem for us in the meantime. Since the open-source and internal codebases are totally out of sync, it's not easy to merge the original PR right now. facebookarchive#1152 Reviewed By: claudiopro Differential Revision: D20016281 fbshipit-source-id: aeefd5c3f17c51c62dd5d7c225cf62d0cd2db26f
vilemj-Viclick
pushed a commit
to kontent-ai/draft-js
that referenced
this pull request
Jul 16, 2020
Summary: Using a Pull Request from github that was made to fix this very bug, I was able to patch the code and fix the problem for us in the meantime. Since the open-source and internal codebases are totally out of sync, it's not easy to merge the original PR right now. facebookarchive#1152 Reviewed By: claudiopro Differential Revision: D20016281 fbshipit-source-id: aeefd5c3f17c51c62dd5d7c225cf62d0cd2db26f
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
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.
Most browsers follow the spec for
compositionendin a reasonable way that conforms to the spec, that is, thedatafield of thecompositionendevent contains the result of the composition. Firefox on Android, however, fires four events in sequence if you select a suggestion for a partially-typed word:compositionend(datais the partial the user typed),beforeinput(datais the partial the user typed),compositionend(datais the remaining letters in the complete word), andbeforeinput(datais the remaining letters in the complete word). As a workaround for nonconforming behaviour like this, this patch changes the handling of events so that a variable textInputData is built from the concatenation of thedatamembers of thebeforeinputevents fired during composition. During resolution, if this string has been built, it is used as the definitive source for the characters that have been typed. Otherwise, thedatamember from the firstcompositionendevent fired during composition is used as the definitive source for the characters that have been typed.I've tested this on Firefox and Chrome under Android 5.x, Safari on the iOS simulator, Chrome and Firefox under iOS, and IE11 under Windows 10. On desktop, I've tested it with both English and Japanese keyboard layouts. I'm reasonably confident that this change will improve support for mobile browser IMEs without breaking compatibility for any other existing supported IME, but I'm very interested in hearing any input that you have wrt these changes.
I acknowledge that I have received permission from Zugata Inc, the company for which I developed these changes under company time, to submit this pull request.