For me this workaround is a hack, but for the moment I have no other clue how to do it. @Bakual any feedback?

Same as you, it's a dirty hack but I don't have a better idea right away.

@Bakual
Why not to skip PlgSystemFields::onContentAfterSave() if $data doesn't have com_fields key?

public function onContentAfterSave($context, $item, $isNew, $data = array())
	{
		// Check if data is an array and the item has an id
		if (!is_array($data) || empty($item->id) || !isset($data['com_fields']))
		{
			return true;
		}
...

Because when you have a checkbox which was checked in the request before and then on the second save unchecked, the com_fields array does not exist, but it is a valid request with custom fields.

Sure, old problem. In this case JForm::filter() can auto-inject empty string for Checkbox/Checkboxes fields.

All code should first filter data, next validate it, next use $data, e.g. see password reset code:

// Filter and validate the form data.
$data = $form->filter($data);
$return = $form->validate($data);

It will help to fix ALL forms with checkboxes as well.

We still face the problem also when we've applied those changes. When the password gets reset
custom fields get emptied.
Is there another bug?

It's same issue with password reset, but it should be fixed with this PR.

@Denitz did it work on your site the password reset with this patch?

@laoneo This PR doesn't solve the user custom fields data lost on password reset. There are few things need to be addressed:

1. There is blank Options tab added to add/edit user screen.

2. The hidden field seems always added to the form (and I guess that's the reason causing the issue on password reset). Maybe you should only insert it if there is custom fields available?

3. Maybe the code if (!$formInput || empty($formInput['processed-by-fields'])) could be shorten to if (empty($formInput['processed-by-fields']))

Please let me know if you need more information.

@joomdonation Please check #18207, it introduces another more solid approach to deal with custom fields data and form checkboxes.

Can you guys test again as I fixed a wrong check. Now the user workarounds should not be needed anymore and the password reset should work too.

maybe i'm testing it in the wrong way

from backend without this pr

1. create a text user custom field
2. create a number user custom field
3. create a new user and fill custom fields data (save it despite it took a lot of times)
4. re-edit the same user, the custom fields data are still there

so i'm unable to replicate the issue
what i'm doing wrong ?

On the the normal user edit form it works, the issue is on password resets as described here #18185 or when you manually execute the code $user = JFactory::getUser(); $user->save();.

Please test #18207, this one is not required if #18207 is accepted.

This one solves a problem we have as soon as you are sending the onContentPrepare event when no form is validated against the data. It fixes the root cause and prevents extension devs from ugly workarounds when saving the data manually trough a model. Yours is a different fix.

@lanoeo i've tested on backend only

As I said, for the normal form it works. Try out #18185. That should erase the custom fields data.

@laoneo
I would disagree: this PR is a crutch, it does not allow to update data without form via model::save() and, it uses hard-coded request values.

It's bad practice to rely on request, e.g. somebody can append jform[processed-by-fields]=1 to query URL and trigger unexpected behaviour. Moreover, request data should not be used in API code like this.

My PR uses logical approach of data update depending on their presence in model data passed to events.

We need someone else with PHP experience to look at this.

Closing because of crutchy code.

Had a second look on it. The problem we are facing is that the plugin save event should not be handled by com_fields when the form is not loaded accordingly. jform[processed-by-fields]=1 has no influence when the form is not validated. Somehow we need to define if the passed data to the save event is processed by com_fields. Otherwise we are not fixing the root cause of the issue.

We should not rely on forms for fields data update, form only displays and provides data to a model. Next model presents data to events. You want to make fields data only updated via form, it's bad because limits model data update.

I would love to not make such a workaround, but to be 100% sure we need to have something which indicates that the data which is sent with the save event was processed before by com_fields. I'm not sure but I guess we have more cases where some data is not sent to the server when it is not empty, radio buttons, select boxes perhaps? With your approach you rely on the fact that the filter function needs to be called first before the save event is triggered which is also not good.
All what this pr here is doing is to add a flag that the saved data is processed by com_fields. Like that we are absolutely on the safe side. While with your approach I have a feeling that we miss some cases.

Form filter function should be called on request data passing data to form validator and next to model, otherwise form rules are not applied. It's correct requirement and it should be always done with request data. That's how JForm works. Otherwise forms rules just don't work. Filtering is essential for forms and we should rely on it.

This flag limits model data save and we can't update model with plain data array. If new model data has a value for a field and the field is allowed to be updates, it should be updated.

As I stated in the description of the pr, this is a workaround. Your approach doesn't convince me because of the fact that I have the feeling we miss there something.

we can't update model with plain data array

What exactly do you mean with that?

I mean that we should be able to easily update certain custom fields via model, without any forms, e.g.

$model->save(....).

And you want to do that with or without custom fields data?

Yes, it would be nice to be able to update custom fields data in this way.

In your pr, can you test, that the following situations do work:

• $user = JFactory::getUser(); $user->save();Should not erase the data.
• Passoword reset is not erasing the data
• If you have one checkbox field and you save the article with it checked and the second time unchecked, that it works.
• Same with radio.
• Same with select box.

If this cases do work, then I'm convinced that we are pretty much on the safe side and I will be willing to close this one.

@laoneo OK to close in favor of PR #18207?

I want to do also another round of testing in #18207 and when all is ok, we can close it. For now I would like to leave it open.

Closing in favor of #18207. Thanks @Denitz.