?
avatar astridx
astridx
20 Jan 2017

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

avatar astridx astridx - open - 20 Jan 2017
avatar joomla-cms-bot joomla-cms-bot - change - 20 Jan 2017
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - labeled - 20 Jan 2017
avatar astridx astridx - edited - 20 Jan 2017
avatar astridx astridx - edited - 20 Jan 2017
avatar Bakual
Bakual - comment - 21 Jan 2017

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
avatar astridx
astridx - comment - 21 Jan 2017

@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.

avatar Bakual
Bakual - comment - 21 Jan 2017

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

avatar astridx
astridx - comment - 21 Jan 2017

@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?

avatar Bakual
Bakual - comment - 21 Jan 2017

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?

avatar laoneo
laoneo - comment - 21 Jan 2017

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

avatar astridx
astridx - comment - 21 Jan 2017

@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?

avatar Bakual
Bakual - comment - 21 Jan 2017

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

avatar laoneo
laoneo - comment - 21 Jan 2017

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

avatar Bakual
Bakual - comment - 22 Jan 2017

For the color formfield see #13677

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 5 Feb 2017

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

avatar Bakual
Bakual - comment - 5 Feb 2017

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.

avatar joomla-cms-bot joomla-cms-bot - change - 5 Feb 2017
Title
[com_fields] Readonly is not working for field types color and editor
[com_fields] Readonly is not working for field types color and editor in combination with disabled
avatar joomla-cms-bot joomla-cms-bot - edited - 5 Feb 2017
avatar franz-wohlkoenig franz-wohlkoenig - change - 30 Mar 2017
Category com_fields
avatar joomla-cms-bot joomla-cms-bot - change - 30 Mar 2017
The description was changed
avatar joomla-cms-bot joomla-cms-bot - edited - 30 Mar 2017
avatar joomla-cms-bot joomla-cms-bot - change - 30 Mar 2017
Title
[com_fields] Readonly is not working for field types color and editor in combination with disabled
[com_fields] Readonly is not working for field types color and editor
avatar joomla-cms-bot joomla-cms-bot - edited - 30 Mar 2017
avatar joomla-cms-bot joomla-cms-bot - change - 30 Mar 2017
The description was changed
avatar joomla-cms-bot joomla-cms-bot - edited - 30 Mar 2017
avatar AlexRed
AlexRed - comment - 5 Apr 2017

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?
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.

avatar Bakual
Bakual - comment - 5 Apr 2017

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.

avatar AlexRed
AlexRed - comment - 5 Apr 2017

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.

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 5 Apr 2017

@AlexRed guessing Documentation is coming up.

avatar Bakual
Bakual - comment - 5 Apr 2017

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.

avatar laoneo
laoneo - comment - 5 Apr 2017

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.

avatar laoneo
laoneo - comment - 5 Apr 2017

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.

avatar brianteeman
brianteeman - comment - 5 Apr 2017

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

avatar AlexRed
AlexRed - comment - 5 Apr 2017

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"

avatar laoneo
laoneo - comment - 5 Apr 2017

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

avatar mbabker
mbabker - comment - 5 Apr 2017

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.

avatar brianteeman
brianteeman - comment - 5 Apr 2017

Exactly!!

avatar franz-wohlkoenig franz-wohlkoenig - change - 5 Apr 2017
Status New Needs Review
avatar joomla-cms-bot joomla-cms-bot - edited - 5 Apr 2017
avatar joomla-cms-bot joomla-cms-bot - edited - 5 Apr 2017
avatar Bakual
Bakual - comment - 6 Apr 2017

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.

avatar dgt41
dgt41 - comment - 6 Apr 2017

@Bakual

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

should do it!

avatar dgt41
dgt41 - comment - 6 Apr 2017

or client side using Joomla's new API:

Joomla.editors.instances['article_text']. setMode('readonly');
avatar Bakual
Bakual - comment - 7 Apr 2017

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.

avatar mbabker
mbabker - comment - 7 Apr 2017

Put it in the params array.

avatar Bakual
Bakual - comment - 7 Apr 2017

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...

avatar Bakual
Bakual - comment - 7 Apr 2017

Please test #15157

avatar Bakual
Bakual - comment - 7 Apr 2017

Closing as we have a PR

avatar Bakual Bakual - close - 7 Apr 2017
avatar Bakual Bakual - change - 7 Apr 2017
Status Needs Review Closed
Closed_Date 0000-00-00 00:00:00 2017-04-07 19:31:35
Closed_By Bakual

Add a Comment

Login with GitHub to post a comment