~eliasnaur/gio#85: 
Editor.SetText during event processing leaves Editor in an inconsistent state that can lead to a panic

I sense the DownArrow key during layout, and set the Editor text via SetText, and them Move to len(theText). If the old text was multiple lines and the new text is a single line, then when the editor's Layout is called immediately afterwards (during the same top-level layout), it too tries to process the DownArrow key and panics, because the editor's lines slice is out of date with respect to the underlying editBuffer (Editor.rr).

It seems to fix the problem (or at least hide it) if I hack Editor.processEvent to abort if !e.valid. But that doesn't seem right — it seems like it might lead to too many events being skipped.

But I'm not really sure, so I'm submitting a bug report instead of a patch.

If I queue the SetText to run after the toplevel layout completes, then that also works, because the editor text and the editorBuffer text are still in sync when the editor tries to process the DownArrow key, and then the text is changed after that. That works around the problem, but I think the core of it is still that SetText leaves Editor.lines and Editor.rr out of sync.

Status
RESOLVED FIXED
Submitter
~theclapp
Assigned to
No-one
Submitted
4 months ago
Updated
4 months ago
Labels
No labels applied.

~eliasnaur 4 months ago

On Mon Feb 3, 2020 at 16:21, ~theclapp wrote:

I sense the DownArrow key during layout, and set the Editor text via SetText, and them Move to len(theText). If the old text was multiple lines and the new text is a single line, then when the editor's Layout is called immediately afterwards (during the same top-level layout), it too tries to process the DownArrow key and panics, because the editor's lines slice is out of date with respect to the underlying editBuffer (Editor.rr).

It seems to fix the problem (or at least hide it) if I hack Editor.processEvent to abort if !e.valid. But that doesn't seem right — it seems like it might lead to too many events being skipped.

But I'm not really sure, so I'm submitting a bug report instead of a patch.

If I queue the SetText to run after the toplevel layout completes, then that also works, because the editor text and the editorBuffer text are still in sync when the editor tries to process the DownArrow key, and then the text is changed after that. That works around the problem, but I think the core of it is still that SetText leaves Editor.lines and Editor.rr out of sync.

Interesting bug. It lead me to discover and fix several flaws in the text layout API,

https://git.sr.ht/~eliasnaur/gio/commit/4c220f45541627cf70144f0f674599bf4a7fa0ef

which then got rid of a long standing hack,

https://git.sr.ht/~eliasnaur/gio/commit/b0af85d3c3771a94f268372f8be318bd3a86d9f2

and finally to a fix for this issue, re-layout text if !e.valid before processing events,

https://git.sr.ht/~eliasnaur/gio/commit/383f3eca40f235e237c3431339cfb6cae007ec17

I'm closing this, please re-open if your problem persists or if I misunderstood it altogether.

~eliasnaur REPORTED FIXED 4 months ago

~theclapp 4 months ago

That does seem to do the trick. Thanks for the quick turnaround!

Register here or Log in to comment, or comment via email.