joomla / joomla-cms

Home of the Joomla! Content Management System
https://www.joomla.org
GNU General Public License v2.0
4.79k stars 3.66k forks source link

[com_fields] You can't submit a form if no permissions to edit a required custom field #31786

Open AndySDH opened 3 years ago

AndySDH commented 3 years ago

Steps to reproduce the issue

Current result

You won't be able to save it, the form will complain that the required field is missing.

It's required but you have no way to submit it because it's disabled for you.

It doesn't even matter if the "Required" field already has a value previously entered (eg. by an Admin). An Editor won't be able to save this article no matter what. Which is even more wrong, because the field even has a value already, yet the form won't submit either:

image

Expected result

You should be able to submit the form even if there is a required field that you have no permissions to edit (whether it's empty or filled).

Additional comments

This already works correctly when "Display When Read-Only" is set to No, because the field is removed from the form.

But if "Display When Read-Only" is set to Yes, then this issue arises.

System information (as much as possible)

Joomla 3.9.23 and all previous Joomla, this has been a longstanding issue.

I haven't tested this in Joomla 4 but I'm pretty sure this is an issue on 4 as well.

Fedik commented 3 years ago

disabled or readonly? these is very different states

AndySDH commented 3 years ago

So the issue may be here: https://github.com/joomla/joomla-cms/blob/62ad0304babd1e87a7433a928676fcee1607b433/administrator/components/com_fields/libraries/fieldsplugin.php#L209

Line 209 should be readonly instead of disabled. Is that it? @Fedik

AndySDH commented 3 years ago

On further inspect, a problem with changing that to 'readonly' would be that chosenJS (which is used by Joomla to output fields in the form) ignores readonly "visually:

There is a plugin for that though: https://www.npmjs.com/package/chosen-readonly Which is based on this simple script that solves it: http://jsfiddle.net/eirc/v2es7L8o/


Also radio fields don't play well with readonly: https://stackoverflow.com/questions/38251473/how-to-make-a-radio-button-read-only-but-not-disabled And they need a CSS tweak to make them "look like" readonly.

input[type="radio"].readonly { opacity: 0.5; /* not necessary */ pointer-events: none; }

But this was actually already fixed for Radio fields, so no problem: see PR https://github.com/joomla/joomla-cms/pull/12039 (minus the opacity thing which would be nice to add)


Either way, using readonly would definitely be better than disabled for the issue reported here.

The problem / side effect of disabled="disabled" is, that it "removes" the field from the data set and it then isn't sent to the server.

Fedik commented 3 years ago

Line 209 should be readonly instead of disabled. Is that it?

Hard to say. The difference that "disabled" not editable and browser does not submit the value, while "readonly" not editable, but browser submit the value.

So if it set disabled but also required then backend validation will fail (I guess js also), and will show "required" error.

AndySDH commented 3 years ago

Yes, exactly, so changing that line I mentioned in my previous comment from disabled to readonlyshould fix the issue.

Or maybe, another approach:

https://github.com/joomla/joomla-cms/blob/62ad0304babd1e87a7433a928676fcee1607b433/libraries/src/Form/Form.php#L2122

Maybe changing line 2122 to if ($required && $fieldExistsInRequestData) could also be a solution.

This would mean that the validation that checks if a field is required would only happen if the field is actually submitted. So disabled fields (which are not submitted) wouldn't be subject to the "required" validation.

Fedik commented 3 years ago

so changing that line I mentioned in my previous comment from disabled to readonly should fix the issue.

Unless it not access check, that will make another issue :wink: I not very know that part.

Maybe there need extra check: if User set field params readonly

bembelimen commented 3 years ago

Disable has the benefit, that Joomla checks for it. So I think the proper solution could be, that the check here should be in an else below (when not disabled)?

Fedik commented 3 years ago

@bembelimen if someone want to use disabled instead of readonly, then value should be preloaded from database (or where it stored) before validateField()

The problem not really in validation, but in logic in general.

obuisard commented 2 years ago

More and more users are reporting this issue. They are getting into very complicated forms using custom fields that may be required fields and read-only. It seems like this is a Joomla 4 issue as well. The only workaround right now is to set the option 'Display When Read-Only' to 'No'.

brianteeman commented 2 years ago

The only workaround right now is to set the option 'Display When Read-Only' to 'No'.

No - the only way is to use the fields correctly. A form with a required field obviously cant be created by a user that you dont give access to that field. You can't have a field that requires input and for it to be readonly. That is a non sequitur.

AndySDH commented 2 years ago

@brianteeman Depends.

The example is provided right exactly in the opening post. Maybe you want a user to edit (not create) an existing article that had a required field already filled by an admin. But you don't want him to be able to edit that one field. Or maybe you want him to edit just the article text and have no permission on the fields at all (but still keep them readonly).

Just like the screenshot in the first post. This is a perfectly valid and correct usage scenario.

And that is not possible right now.