Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

marker#deleteValueAtOffset detects if the character is outside the BMP #275

Merged
merged 1 commit into from
Jan 6, 2016

Conversation

bantic
Copy link
Collaborator

@bantic bantic commented Dec 18, 2015

  • acceptance tests for forward- and backward-deleting emoji
  • audit the code for other places that may be affected

There remain some potential places where markers can still be affected by multi-code-point characters. Anywhere in models/marker.js that uses the native string-mutation methods (substr, e.g.) on this.value are affected, but deleteValueAtOffset is the only one that will typically trip on this. The other methods will normally be dealing with selections read from the DOM and browsers should not allow a user to select 1/2 of an emoji.

A more comprehensive fix is to write a set of UCS-2 safe string mutation functions and replace every use of built-in string mutation functions in models/marker.js with those.

fixes #274

@mixonic
Copy link
Contributor

mixonic commented Dec 18, 2015

👏 👏 👏 👏 👏

@bantic bantic force-pushed the fix-marker-emoji-deletion-274 branch 2 times, most recently from 95098c9 to 2655232 Compare January 6, 2016 20:47
@bantic bantic force-pushed the fix-marker-emoji-deletion-274 branch from 2655232 to f3b72ae Compare January 6, 2016 20:51
bantic added a commit that referenced this pull request Jan 6, 2016
marker#deleteValueAtOffset detects if the character is outside the BMP
@bantic bantic merged commit b662def into master Jan 6, 2016
@bantic bantic deleted the fix-marker-emoji-deletion-274 branch January 6, 2016 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

deleting characters outside the BMP does not work
2 participants