- The test now ensures that all three key events are fired when
sending keys. Previously, only the 'keypress' event was sent,
which failed to trigger very slow code on webkit browsers (as
it is triggered by 'keyup'). All three events should really be
sent whenever sending keys to the browser to ensure that we're
adequately testing real behavior. See the 'sendkeys' plugin
for more; it only sends 'keypress'.
- Do not use incorpIfQuick on keyup because it's not ever quick
on chrome. Calling incorpIfQuick calls incorporateUserChanges
which sets a flag on the current callstack state indicating
that the selection has changed (since something was typed).
Whenever this flag is set, the event handler will run code
to update the selection (and possibly scroll the view as
well), which is a very costly operation in webkit browsers.
Instead let the user changes be incorporated by the idle worker,
scheduling it to run ASAP on keyup. This isn't a perfect
solution, but may make pads running on webkit browsers more
usable with otherwise fairly unnoticeable changes in the UI.
THE BUG - HIGH LEVEL:
- When client A sends out an attribute change, client B applies that change to itself but
also thinks that it made the change itself, which is incorrect. This means that when client B
next makes a change, he will send out that he made the attrib change that A actually made.
- Ex: Have 2 clients on the same pad. Have A apply bold on some text. Next, have B type a character.
B will broadcast that it both added a character AND applied bold, when in reality it did NOT
apply bold at all, that change was done by the other client and this client incorrectly adopted it as its own.
- This root bug behavior results in clients continuing to think that they each made the other client's change,
thus resulting in an infinite loop of changeset spamming that bogs down clients and harms server stability.
THE BUG - IN DEPTH:
- The root issue is in the way that Changesets are combined in Changeset.follow(). Specifically, in the case of a
changeset with ONLY new attrib changes (no text changes) being merged with an identity changeset (has no ops).
- In this case, Changeset.follow() copies the ops of the new CS and fully overrides the other CS.
- applyChangesToBase invokes Changeset.follow to determine the final client document CS state after applying the new CS.
If the final client document CS state is NOT the identity CS, then the client broadcasts that it made a change.
- When client A changes just attribs, client B's applyChangesToBase calls Changeset.follow() and passes client A's
changeset (attrib change) and Client B's current changeset state (identity).
- As per the noted bug, Changeset.follow() returns client A's changeset as a result, causing client B to adopt
client A's changeset as its own document state. Thus, client A ends up thinking it has made client B's changes.
THE FIX:
- Changeset.follow() should NOT copy the ops of the new CS passed in if those changes are only attrib changes.
This allows applyChangesToBase to properly set the client's CS back to the identity after applying an
external attrib change, instead of incorrectly adopting the external client's changes.
John McLear