Right now, the ghost text feature is quite incompatible with undo/redo support. Since we weren't filtering out ghost text from a text input's contents when registering undoable actions, we were susceptible to a bunch of weird side effects, such as ghost text becoming editable or even crashes.
The fix here is to make sure any undoable actions ignore ghost text. Ghost text is very transient and is designed to disappear as soon as you make any sort of change, so the easiest way to do this is to just remove it directly before doing anything.
A related issue pops up where a write to setAttributedText: also triggers a call to set the ghost text to nil. Since setAttributedText: already gets rid of the ghost text when we ask it to, the subsequent call to setGhostText: may not need any action. We solve this by being more lenient in this case, assuming that if the ghost text isn't in where our internal state expects it to be, we probably already removed it.
Test Plan:
Validated that ghost text behaves properly against the following scenarios:
Modify the value of the text input by typing manually.
Setting the value of the text input on the JS side.
Moving the cursor via a mouse click or arrow keys.
Placing ghost text either in the middle or at the end of the text.
Cases where the ghost text immediately precedes or follows "real" text with the same value (e.g., "This is ghost(ghost) text" or "This is (ghost)ghost text").
Summary:
Right now, the ghost text feature is quite incompatible with undo/redo support. Since we weren't filtering out ghost text from a text input's contents when registering undoable actions, we were susceptible to a bunch of weird side effects, such as ghost text becoming editable or even crashes.
The fix here is to make sure any undoable actions ignore ghost text. Ghost text is very transient and is designed to disappear as soon as you make any sort of change, so the easiest way to do this is to just remove it directly before doing anything.
A related issue pops up where a write to
setAttributedText:
also triggers a call to set the ghost text tonil
. SincesetAttributedText:
already gets rid of the ghost text when we ask it to, the subsequent call tosetGhostText:
may not need any action. We solve this by being more lenient in this case, assuming that if the ghost text isn't in where our internal state expects it to be, we probably already removed it.Test Plan:
Validated that ghost text behaves properly against the following scenarios: