Skip to content

Commit

Permalink
Fixed autocorrect causing dupe text (ianstormtaylor#655)
Browse files Browse the repository at this point in the history
* Refactored out `getPoint` from components/content to utils so as to make it more reusable.

* Fixed autocorrect causing dupe text. (ianstormtaylor#540)

During autocorrection (in iOS’s Safari), `onSelect` event is triggered after `onBeforeInput` event.
The plugins/core updates the state during `onBeforeInput` event, thus causing selection triggered by autocorrect to be lost/overridden.
This behaviour caused dupe text bug during autocorrection.

To overcome this issue, we try to query the selection and conditionally fix out of sync cases with an additional transform before inserting the text.

* Removes Content#getPoint and use the new utility function instead.

* Renames local variable nextTransform to transform.

* Describe the solution to the autocorrect issue in a more descriptive manner.
  • Loading branch information
lxcid authored and Yalu committed Apr 16, 2017
1 parent 2818506 commit 6e9580c
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 40 deletions.
46 changes: 8 additions & 38 deletions src/components/content.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import Base64 from '../serializers/base-64'
import Debug from 'debug'
import Node from './node'
import OffsetKey from '../utils/offset-key'
import getPoint from '../utils/get-point'
import React from 'react'
import ReactDOM from 'react-dom'
import Selection from '../models/selection'
Expand Down Expand Up @@ -139,36 +139,6 @@ class Content extends React.Component {
}
}

/**
* Get a point from a native selection's DOM `element` and `offset`.
*
* @param {Element} element
* @param {Number} offset
* @return {Object}
*/

getPoint(element, offset) {
const { state, editor } = this.props
const { document } = state
const schema = editor.getSchema()

// If we can't find an offset key, we can't get a point.
const offsetKey = OffsetKey.findKey(element, offset)
if (!offsetKey) return null

// COMPAT: If someone is clicking from one Slate editor into another, the
// select event fires two, once for the old editor's `element` first, and
// then afterwards for the correct `element`. (2017/03/03)
const { key } = offsetKey
const node = document.getDescendant(key)
if (!node) return null

const decorators = document.getDescendantDecorators(key, schema)
const ranges = node.getRanges(decorators)
const point = OffsetKey.findPoint(offsetKey, ranges)
return point
}

/**
* The React ref method to set the root content element locally.
*
Expand Down Expand Up @@ -424,7 +394,7 @@ class Content extends React.Component {
event.preventDefault()

const window = getWindow(event.target)
const { state } = this.props
const { state, editor } = this.props
const { nativeEvent } = event
const { dataTransfer, x, y } = nativeEvent
const data = getTransferData(dataTransfer)
Expand All @@ -441,7 +411,7 @@ class Content extends React.Component {
}

const { startContainer, startOffset } = range
const point = this.getPoint(startContainer, startOffset)
const point = getPoint(startContainer, startOffset, state, editor)
if (!point) return

const target = Selection.create({
Expand Down Expand Up @@ -481,16 +451,16 @@ class Content extends React.Component {
debug('onInput', { event })

const window = getWindow(event.target)
const { state, editor } = this.props

// Get the selection point.
const native = window.getSelection()
const { anchorNode, anchorOffset } = native
const point = this.getPoint(anchorNode, anchorOffset)
const point = getPoint(anchorNode, anchorOffset, state, editor)
if (!point) return

// Get the range in question.
const { key, index, start, end } = point
const { state, editor } = this.props
const { document, selection } = state
const schema = editor.getSchema()
const decorators = document.getDescendantDecorators(key, schema)
Expand Down Expand Up @@ -652,7 +622,7 @@ class Content extends React.Component {
if (!this.isInContentEditable(event)) return

const window = getWindow(event.target)
const { state } = this.props
const { state, editor } = this.props
const { document, selection } = state
const native = window.getSelection()
const data = {}
Expand All @@ -666,8 +636,8 @@ class Content extends React.Component {
// Otherwise, determine the Slate selection from the native one.
else {
const { anchorNode, anchorOffset, focusNode, focusOffset } = native
const anchor = this.getPoint(anchorNode, anchorOffset)
const focus = this.getPoint(focusNode, focusOffset)
const anchor = getPoint(anchorNode, anchorOffset, state, editor)
const focus = getPoint(focusNode, focusOffset, state, editor)
if (!anchor || !focus) return

// There are valid situations where a select event will fire when we're
Expand Down
34 changes: 32 additions & 2 deletions src/plugins/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Base64 from '../serializers/base-64'
import Content from '../components/content'
import Character from '../models/character'
import Debug from 'debug'
import getPoint from '../utils/get-point'
import Placeholder from '../components/placeholder'
import React from 'react'
import getWindow from 'get-window'
Expand Down Expand Up @@ -97,9 +98,38 @@ function Plugin(options = {}) {

const chars = initialChars.insert(startOffset, char)

let transform = state.transform()

// COMPAT: In iOS, when choosing from the predictive text suggestions, the
// native selection will be changed to span the existing word, so that the word
// is replaced. But the `select` event for this change doesn't fire until after
// the `beforeInput` event, even though the native selection is updated. So we
// need to manually adjust the selection to be in sync. (03/18/2017)
const window = getWindow(event.target)
const native = window.getSelection()
const { anchorNode, anchorOffset, focusNode, focusOffset } = native
const anchorPoint = getPoint(anchorNode, anchorOffset, state, editor)
const focusPoint = getPoint(focusNode, focusOffset, state, editor)
if (anchorPoint && focusPoint) {
const { selection } = state
if (
selection.anchorKey !== anchorPoint.key ||
selection.anchorOffset !== anchorPoint.offset ||
selection.focusKey !== focusPoint.key ||
selection.focusOffset !== focusPoint.offset
) {
transform = transform
.select({
anchorKey: anchorPoint.key,
anchorOffset: anchorPoint.offset,
focusKey: focusPoint.key,
focusOffset: focusPoint.offset
})
}
}

// Determine what the characters should be, if not natively inserted.
let next = state
.transform()
let next = transform
.insertText(e.data)
.apply()

Expand Down
40 changes: 40 additions & 0 deletions src/utils/get-point.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import OffsetKey from './offset-key'

/**
* Get a point from a native selection's DOM `element` and `offset`.
*
* @param {Element} element
* @param {Number} offset
* @param {State} state
* @param {Editor} editor
* @return {Object}
*/

function getPoint(element, offset, state, editor) {
const { document } = state
const schema = editor.getSchema()

// If we can't find an offset key, we can't get a point.
const offsetKey = OffsetKey.findKey(element, offset)
if (!offsetKey) return null

// COMPAT: If someone is clicking from one Slate editor into another, the
// select event fires two, once for the old editor's `element` first, and
// then afterwards for the correct `element`. (2017/03/03)
const { key } = offsetKey
const node = document.getDescendant(key)
if (!node) return null

const decorators = document.getDescendantDecorators(key, schema)
const ranges = node.getRanges(decorators)
const point = OffsetKey.findPoint(offsetKey, ranges)
return point
}

/**
* Export.
*
* @type {Function}
*/

export default getPoint

0 comments on commit 6e9580c

Please sign in to comment.