Document that #entity_builders and overrides of EntityForm::copyFormValuesToEntity() must be idempotent

(Hoping to indirectly trigger some additional insight or jolt a memory by providing my experience with this.)

I know I've struggled with this one too. I forget in which exact context — it was either:

1. in relation to the Quick Edit module
2. in relation to the work I did on making forms not store their state always by default (which you also worked on)
3. while working on debugging some AJAXy things: the entire form state is being rebuilt always, nothing is actually ever retrieved from form state

So, from what I remember: there is something extremely fishy going on with EntityForm::init() and EntityForm::prepareEntity(). Basically, this is a lie:

  /**
   * Prepares the entity object before the form is built first.
   */
  protected function prepareEntity() {}

It definitely doesn't run only when the form is first built.

EDIT: I think I remember where I saw the AJAXy fishiness: while debugging/patching something related to the "Text formats & editors" configuration UI: there is an AJAXy text editor <select>, which causes an AJAX form to be loaded. There have been multiple issues related to that, and there's one remaining one: #2701393: Switching between editors on the format configuration causes errors upon save.

Again, sorry if this is a distraction. Just trying to help you make the connection to other issues.

what if...

Kind of expecting everything to explode, but maybe not?

Well, I guess because of the clone reference problem, arguably there is not that much difference at the moment between setting $this->entity or not, because the field values will be the same anyway.

So any bug will be masked in the same way the form language problem is currently masked.

Trying out the fix for the cloning plus the patch from #12 (the build-twice one).

The problem with the cloning only occurs when the entity has more than one translation otherwise not, so the problem will not be masked for each case.

Having thought about this a lot for the past couple of days, I now relaize the issue title reveals quite precisely why ContentEntityForm::updateFormLangcode() is problematic (as it is an #entity_builder: it's not idempotent! It sets a value into the form state. So regardless of whether the built entity gets thrown away or set into $this->entity, the form state now has a different language code. That is a problem IMO.

See \Drupal\user\AccountForm::buildEntity() for an interesting example. We are changing form state there as well, we just have to prevent that we do the same multiple times and break it the second time.

And yes, that is the only way I could find to do what we need to to there. Would be much easier if roles were a widget.

@berdir - \Drupal\user\AccountForm::buildEntity() only transforms the user submitted values for the roles in a acceptable way for the entity "roles" field and this form state value is only used later to set it on the entity field. So it basically does not alter the state of the form. It basically does what WidgetBase::extractFormValues would do.

However as you've said it would be easier if roles were a widget. So why don't we provide a widget for the roles field or am I missing something?

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Lowering to normal after 6 years not sure if it's a major issue.

The last remaining task is to document this but I don't see any documentation?

Could someone please take a look