Druid: Numeric textboxes break when given non-numeric data in recent git revisions

[bheisler]

Run cargo run --example parse. Click on text box. Press Backspace, followed by any letter key.

Expected behavior: The 0 in the text box is deleted, the letter appears in the text box.
Actual behavior: The 0 in the text box is deleted correctly. The letter never appears. The text box now ignores all further key and mouse input. Warning messages such as the following appear in the log on all input, whether keyboard or mouse.

WARN [druid::text::editor] editor data changed externally, skipping event Insert("h")

Known working commit: b38e873f7fceff5068c11e950aa2202b472729a2
Known broken commit: 694af59d63318c7197f81eedba56de3071182321
Git bisect says first broken commit is c64372879de3e28cc2b7f1deeb97352362f8f2be, though the exact behavior of these broken text boxes varies by commit.

Notably, this does _not_ occur on text boxes which are bound to String fields.

I can also rule out a regression in the Parse type itself, since I've written my own independent, extended alternative to Parse and it displays the same bug. This seems to be related to recent work on TextBox.

[cmyr]

Thanks! a bunch of the text editing stuff is in flux right now, so not too surprising we shake out some bugs. Should get this fixed shortly.

[bheisler]

I read over your commit and tried to recreate it on my cloned Parse widget. A couple of things:

• I'm not sure what the inner data vs outer data comment means.
• It looks like this change requires the textbox to be updated even if the string didn't change, have I understood that correctly? If the TextBox is wrapped in a WidgetPod, though, that seems to be impossible; WidgetPod's update method only takes the new data and performs its own check if the data has changed. I tried forcing the issue by calling the update with an artificial fake value but it didn't seem to work. Should it? I suppose I'm doing some esoteric custom-widget stuff, but this seems like a footgun.

Maybe I should take a step back here and explain - I cloned the Parse type because Parse doesn't give me any access to the parse error or any way to display it to the user. My clone of the Parse type displays the error in a TextLayout beneath the TextBox, but this text only becomes visible if the input in the textbox is invalid. But that means I need the TextBox to only take up part of the bounds of the overall widget (else it overlaps with the error text). I couldn't find any way to make that work (it really seems like using paint.with_child_ctx(region, |child_ctx| { ... }); should do that, but it didn't) except to take a cue from the Padding widget and wrap the contained TextBox in a WidgetPod and use set_layout_rect to force the TextBox to the right size. Which in turn means that I now can't force it to update even if the data has changed.

The code for my BetterParse widget is here, if you'd like to offer suggestions: https://gist.github.com/bheisler/1007a054336ce5c08cb9d5c3a0e39f0e

[cmyr]

My opinion is that parse as it currently exists is 100% a placeholder, and is not a robust long-term solution. I think its interactions with the text box are part of this. I've played around with something kind of like this in runebender, the EditableLabel widget: https://github.com/linebender/runebender/blob/master/src/widgets/editable_label.rs

some other thoughts:

• I'm curious about what the problem was with with_child_ctx? That should work, I think.
• It's true that adding in a WidgetPod could cause a problem here. The issue is that TextBox owns a TextLayout, and updates it when edits are made. So what happens is that when an 'invalid' edit occurs in the TextBox, its inner state holds whatever text was entered; but the parse widget rejects that edit, and it's state doesn't change, which means it is now different from the state of the TextBox. We need some way of telling the TextBox that its change was rejected, and that there is 'new' (actually old) text.
• the correct solution, to me, is to have the validation logic be more tightly coupled with the TextBox; this means some kind of controller that owns the textbox and also owns the validation/parse code, and which explicitly ensures that the state is correct in both places?
  The widget pod situation is interesting.

[RaoulHC]

Your fix for this in #1294 can now cause a panic for other types such as floats, where there are states where parse followed by to_string doesn't give you the same result. For the parse example with f32 instead of u32, "1." in the text box becomes "1" in the parse state here, and then when you try and insert an additional character the selection is out of bounds causing a panic.

I would have thought that the state in parse should just be the value in the text box, as this would prevent any mismatches in state and keep the textbox behavior as is.
I'm not too familiar with how widgets fit together though, so I might be missing something.

[cmyr]

@RaoulHC Want to check that #1312 works as expected?