fix: treat "clear all" as permanent initial state

[denis-anisimov]

fixes #10119

[knoobie]

I'm wondering by this test name.. please ignore if totally out of scope. just wondering. Does that mean the following:

// already attached to the client
testForm.getDiv().setText("Bar");
// already attached client side is updated to "Bar"

// remove TestForm from client
// reattach TestForm to client

Does TestForm has "Template text" or "Bar" on the client? From the test name I would expect "Template text" but from my "state model understanding" I would think it should be "Bar"?

[denis-anisimov]

There is no "Bar" in the this scenario.

The original ticket is about duplicated text.
So "Template text" will be duplicated after reattach.

There is an implicit call setText("Template text").

May be you are right that I should avoid implicit setText and call explicitely setText("foo" and check that this is shown in the result...

[denis-anisimov]

Done.

[knoobie]

Thanks that makes the intention more clear and additionally checks that the state is stored in between attachments and not "overwritten" when reattached.

[mshabarov]

I'm not certain whether we need to update the javadoc for generateChangesFromEmpty().
From a modified implementation point of view, it now adds a clear change even if the values are empty.
But Denis's explanations looks reasonable.

[mshabarov]

When that instance is then detached in one round trip and attached again in another round trip, then without this PR it will show the "Hello" text because there will be a new client-side instance that runs firstUpdated again but there won't be any clear event from the server. With this PR, a clear event is sent which leads to an empty element.

Apart from the knowledge on how the state nodes work under the hood, I would say that having empty text after first attach, and, then, having "Hello" text after subsequent attach/reattach, would be confusing and opens a gates for bugs, and, as Denis said, a faulty costumer-side code which relies on that behaviour. On the other hand, having the clear all state constantly looks more reasonable, because this behaviour at least expectable.

I think it should be just mentioned in the release notes.

Yes, and I believe we shouldn't maybe release this fix in the flow 2.5 immediately, but, instead, have some test period in latest flow versions to be sure that this would not cause a new regressions.