HomePhabricator

Followup 2fcc4c0: fix bugs with selection-only polling

Authored by Catrope.

Description

Followup 2fcc4c0: fix bugs with selection-only polling

The optimizations in selection-only polling were too aggressive.
When the cursor moved to a different node and the next poll was
a selection-only poll, we would not update this.node because that
code path was skipped for selection-only polls. Changing the
selection by clicking with the mouse will cause both a
selection-only poll (from onDocumentSelectionChange) and
a full poll (from a timeout in onDocumentMouseDown) to occur,
but they can occur in either order, at least in Chrome.
Probably what's going on is that there may or may not be
a timer tick between mousedown and selectionchange, depending
on how fast the user clicks or something.

I think this is related to recent cases I've seen of inserted
characters at the end of a line ending up on the next line.
I would imagine something like this could have caused that:

  • User clicks at end of line
  • mousedown happens, sets timeout for full poll
  • DOM selectionchange event happens, does selectionOnly poll
  • Poll emits selectionchange event but does not update this.node
  • Timeout fires, full poll happens
  • Poll observes that DOM selection hasn't changed, does nothing
  • User types 'A', causes full poll
  • Poll observes this.node has changed, does not check for content change
  • No contentChange emitted, 'A' not added to DM, DM and CE out of sync
  • Subsequent polls do nothing because DOM selection didn't change
  • User types again
  • Poll observes 'B' was added, emits contentChange
  • CE sees <p>HelloA|B|</p>, computes insertion offset 7
  • DM doesn't have 'A', so "insert B at 7" is <p>Hello</p>|B|
  • This is normalized to <p>Hello</p><p>B</p> and committed
  • New paragraph is rendered but old paragraph isn't rerendered
  • CE DOM now shows <p>HelloAB</p><p>B</p>
  • User is confused and goes to Roan's desk

Change-Id: I6333590e46c6c7241b8974afe5e5dfed9d79c877