joomla-cms
joomla-cms copied to clipboard
[5.3] Carry through disabled attribute to sub-fields in media and subform custom fields
Pull Request for Issue #38600 .
Summary of Changes
Added some checks in the plugins and layout fields to see if the parent fields are set as disabled. If so, set subfields to disabled also.
~~This is IN PROGRESS, feedback welcome.~~ Ready for review!
~~Left to do:~~ Done!
- [x] Something funny is happening with disabled subforms that have media fields when you save them.
- [x] Make sure buttons on repeatable subforms are disabled or hidden when subform is disabled.
Testing Instructions
- Make sure you have two users with different user groups, both with edit access to articles
- As superuser, create a custom field of type "media".
- In the "options" tab, set "display when read only" to "yes"
- In the "permissions" tab, set the edit permissions for your other user to denied.
- As superuser, create a custom field of type "subform" with at least one subfield.
- In the "options" tab, set "display when read only" to "yes"
- In the "permissions" tab, set the edit permissions for your other user to denied.
- Login as the other user and navigate to the edit form where the custom fields are visible.
Actual result BEFORE applying this Pull Request
Media and subform fields are visible AND can be edited, or appear to be editable. Rows can be added and removed. Fields can be changed and text can be entered. However, any changes will not be saved to the database.
Expected result AFTER applying this Pull Request
Media and subform fields are visible but cannot be edited. Fields for media select, alt text, etc are disabled. Rows cannot be added or removed.
Documentation Changes Required
None to my knowledge.
This is as far as I can get it with my own skills. I'm not sure why the media field in disabled subforms breaks things so astonishingly.
Here's what I'm experiencing:
- disabled subform already has content in media field and other fields. User cannot edit it, as expected with this PR.
- Save the article
- Any media fields in the disabled subform disappear from the table, and there is a warning:
Invalid field:[Media field name] - The form looks broken, it's scary, refreshing the page doesn't help.
- Closing the article and opening it again restores the previous un-broken state because nothing was actually saved.
All the strange edge cases and bugs (looking at you, accessible media field...) have now been resolved to the best of my knowledge. Ready for testing!
Roland @roland-d do you want to move this one to 4.3 or include in in 4.2?
@obuisard This is fine for 4.2 given we have enough tests and all conversations are resolved.
Please fix conflicts.
There you go @Quy , sorry for the delay!
This pull request has been automatically rebased to 5.0-dev.
Hey @HLeithner and @Hackwar , this is a bug fix regarding access/permissions issues, not a feature. Is it possible to move this back to 4.3? It's pretty much ready though I see now I need to check why the build is failing.
I rebased it to 4.3-dev, not changing the label, RM for 4.3 should update the label if it's a bug.
Thank you Harald! @obuisard and @sdwjoomla , what are your thoughts?
Crystal @crystalenka, I think that is a UI/UX bug, so I would like to see it carried through 4.3. However, we will need to adjust the code to rid of the conflicts
Agree with Olivier this is a bug as it does not function as expected
Crystal @crystalenka can you look at the conflicts? Let us know if you need help with that. Thanks!
Thank you! It's on my list and I will check it as soon as I can.
No pressure Crystal @crystalenka, just want to make sure you did not forget about it :-) Would love to see the fix in 4.3.3.
Hi @obuisard , sorry for taking so long. I believe I fixed the conflicts now.
Hi @obuisard , sorry for taking so long. I believe I fixed the conflicts now.
Thank you very much Crystal @crystalenka. That will help get it tested :-)
Log in as a non super user. Edit an article. See console.
This pull request has been automatically rebased to 4.4-dev.
This pull request has been automatically rebased to 5.2-dev.
Is this PR still necessary? If yes, can you please fix the conflict?
I've allowed myself to fix the merge conflict.
I have tested this item :white_check_mark: successfully on 5962ee04a751f53d5482aeb4a72696f9e822162b
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38602.
I have not tested this item.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38602.
I have tested this item :white_check_mark: successfully on 5962ee04a751f53d5482aeb4a72696f9e822162b
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38602.
I have tested this item :white_check_mark: successfully on 5962ee04a751f53d5482aeb4a72696f9e822162b
I had tested it is working as expected.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38602.
Not setting RTC because there are still updated requested by reviewers, see https://github.com/joomla/joomla-cms/pull/38602#issuecomment-1642567944 and https://github.com/joomla/joomla-cms/pull/38602#discussion_r1283842259 .
This pull request has been automatically rebased to 5.3-dev.
This pull request has been automatically rebased to 5.4-dev.