[com_fields] Readonly is not working for field types color and editor

[astridx]

Steps to reproduce the issue

Create a field of type color or of type editor and assign the option read-only. You can find this option on the register options.

Expected result

When I create an article the field should be read-only

Actual result

It is possible for me to edit this fields.

System information (as much as possible)

Additional comments

[Bakual]

That's actually not an issue of com_fields by itself.
Those formfields just don't support readonly/disabled attributes.

There is an easy "fix" for the respective field plugins in that we just remove the "readonly" parameter, however it would still have the issue that ACL wouldn't work for those types.
The better fix would be to add readonly/disabled functionality to the formfields itself:

• libraries/joomla/form/fields/color.php
• libraries/cms/form/field/editor.php

[astridx]

@Bakual Thank you for explaining. I will have a closer look to the easy "fix" today.
Because I have further issues. One of this: A disabled field is validated. If I disable a required field, I am not able to save an article. I get the message, that I have to fill the requiered field. But as I understand the w3c https://www.w3.org/TR/html5/forms.html#constraint-validation required fields should not be validated.

[Bakual]

On the other hand having a required field which isn't available for editing is quite useless, Maybe better don't show it then?

[astridx]

@Bakual You are right. In Joomla! it would be better to un-publish this field. But it is confusing for people who are knowing the html5 rules and who are new in Joomla!. They would expect the “normal” field behavior. At least we need to document it.
And if I think further why do we need the option disabled. Could we remove this option?

[Bakual]

And if I think further why do we need the option disabled. Could we remove this option?

@laoneo you know the use case behind that?

[laoneo]

It's basically a supported field in HTML forms to show the user the value of the field, but that it's not editable.

[astridx]

@laoneo
And the value of the disabled field should not be transported if the form is submitted. But at the Moment a disabled custom field transports the value und validated it.
In my opinion this is confusing for the user.
And in Joomla we do not need to disable a field because unpublishing does the same.

So can we remove the field?

[Bakual]

Imho, we should fix the formfields, should be easy for the color one. The editor one may be a bit tricky.

[laoneo]

Do whatever you think is best as you are not the first one raising this question

• Feedback: What is the difference between Disable Field and Unpublish ? joomla-projects/custom-fields#92
• Disabled and Read Only option joomla-projects/custom-fields#145

[Bakual]

For the color formfield see #13677

[ghost]

so theres a tricky fix on the editor-formfield open.

[Bakual]

Best bet is probably to return a simple textarea (return parent::getInput()) if either readonly or disabled is set. I don't think the editors itself support those.

[AlexRed]

Also the User editor field (and contact Mail editor field) is editable with ACL set to no edit. But the User editor field not work, no data saved if ACL set to no edit.
Why display a field in the User registration form or in the Contact Mail form if it is not editable or it is editable but not work?
But it can be "required" and block the procedure.
I guess we will get tons of requests why a field is visible but no editable, just because the permission is wrong set (by default).
So for me an ACL "column" in the field list is important, to show if the field can be edit or no by the public. By default all new field are set to no edit in the ACL but the administrator can't know it, not a good user experience.

[Bakual]

As for the default ACL settings, that's a different issue. You can see the (closed) issue here: #14336
That will be a question of documenting that usecase. It's not something we can (or should) fix in code imho.

So for me an ACL "column" in the field list is important, to show if the field can be edit or no by the public. By default all new field are set to no edit in the ACL but the administrator can't know it, not a good user experience.

It is shown in the permission tab, so the administrator can know it. Honestly, custom fields will have some learning curve associated with it. It's not hard to use, but there are some tricks you will have to know when working with it. Documentation is key here.

[AlexRed]

Sorry but no Documentation about ACL permission for custom field :(
For me it is a bug: the User editor field (and contact Mail editor field) is editable with ACL set to no edit.

[ghost]

@AlexRed guessing Documentation is coming up.

[Bakual]

Sorry but no Documentation about ACL permission for custom field :(

Custom fields aren't released yet, documentation is still a work in progress. Hopefully someone also writes something about ACL there.

For me it is a bug: the User editor field (and contact Mail editor field) is editable with ACL set to no edit.

Yep, there is a bug that the editor doesn't support readonly/disabled, that is what this issue here is about. Unfortunately nobody stepped up to add that to the editor formfield yet.
My comment before was about the default ACL values which would be a diffreent issue.

[laoneo]

It is a bug in the editor and that should not be solved with a particular setting which nobody will understand. The point is that the bug in the editor must be solved and not done some workarounds.

[laoneo]

If you think we should hide the fields instead of showing them disabled when they are not editable then please open an issue for that.

[brianteeman]

Hiding them just creates a different support issue. "I added fields and they don't display"

[AlexRed]

yes but solve the issues with the editor and also now you have the same support issue. "I added fields and they are not editable"

[laoneo]

But should we then not try to fix the editor instead of workarounding things?

[mbabker]

CodeMirror has a readOnly option - https://codemirror.net/doc/manual.html
TinyMCE can be set to a read only mode - https://www.tinymce.com/docs/api/tinymce/tinymce.editor/#setmode
"None" editor is just a textarea, HTML disabled should work

If the editor form field, editor plugins, and JEditor can't somehow enable these, that is the bug that needs to be fixed.

[brianteeman]

Exactly!!

[Bakual]

I had a look if I can get the editors into readonly mode. For codemirror I was able to set that flag hardcoded in the editor plugin, for TinyMCE I couldn't figure out yet where I would have to set it.
But then that isn't the biggest problem anyway. The way the editors work, the editor plugins are completely disconnected from the editor formfield. They go through JEditor and only a very limited set of parameters are passed over from the form to the plugins. Readonly/Disabled states are not part of it.
I couldn't figure out a nice way of doing it.
So while the JFormfieldEditor actually would respect the disabled and readonly flags, they just don't get passed through.
If someone is more familiar with how our editors work, feel free to jump in here.

[dgrammatiko]

@Bakual

tinyMCE.init({
        ...
        theme : "advanced",
        readonly : 1
});

should do it!

[dgrammatiko]

or client side using Joomla's new API:

Joomla.editors.instances['article_text'].instance.setMode('readonly');

[Bakual]

But how does the plugin know when to set it? It doesn't know anything about the formfield and the formfield doesn't know anything about the editor.
The current call goes through JEditor::display() (see https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/editor/editor.php#L289) which then passes the arguments to the editor plugin here https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/editor/editor.php#L310-L320.
I don't want to add two more arguments there, it is already ridiculous as it is.

[mbabker]

Put it in the params array.

[Bakual]

The params array unfortunately isn't passed to the editor plugin display method neither. It's only used to initially instantiate the editor, which is only done once per pageload.
The longer I look at that thing the more I don't understand it...

The codemirror onDisplay method has this signature:
public function onDisplay($name, $content, $width, $height, $col, $row, $buttons = true, $id = null, $asset = null, $author = null, $params = array())

However when you look at what is passed from JEditor to this event it is:

$args['name'] = $name;
$args['content'] = $html;
$args['width'] = $width;
$args['height'] = $height;
$args['col'] = $col;
$args['row'] = $row;
$args['buttons'] = $buttons;
$args['id'] = $id ?: $name;
$args['event'] = 'onDisplay';

$asset, $author and $params aren't even passed to the method and they are also unused in the method (not so surprisingly).
So if I wanted to pass an additional $readonly argument to the method, I would have to add 3 other unused variables first to the call in JEditor. Even if we use the $params array, I need to pass $asset and $author as well first.
I feel dirty just thinking about that...

[Bakual]

Please test #15157

[Bakual]

Closing as we have a PR