joomla-cms icon indicating copy to clipboard operation
joomla-cms copied to clipboard

[5.3] Carry through disabled attribute to sub-fields in media and subform custom fields

Open crystalenka opened this issue 3 years ago • 28 comments

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.

crystalenka avatar Aug 26 '22 08:08 crystalenka

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.

crystalenka avatar Aug 26 '22 08:08 crystalenka

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!

crystalenka avatar Oct 28 '22 18:10 crystalenka

Roland @roland-d do you want to move this one to 4.3 or include in in 4.2?

obuisard avatar Jan 13 '23 23:01 obuisard

@obuisard This is fine for 4.2 given we have enough tests and all conversations are resolved.

roland-d avatar Jan 14 '23 09:01 roland-d

Please fix conflicts.

Quy avatar Jan 23 '23 23:01 Quy

There you go @Quy , sorry for the delay!

crystalenka avatar Feb 07 '23 09:02 crystalenka

This pull request has been automatically rebased to 5.0-dev.

HLeithner avatar May 02 '23 16:05 HLeithner

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.

crystalenka avatar May 02 '23 16:05 crystalenka

I rebased it to 4.3-dev, not changing the label, RM for 4.3 should update the label if it's a bug.

HLeithner avatar May 02 '23 16:05 HLeithner

Thank you Harald! @obuisard and @sdwjoomla , what are your thoughts?

crystalenka avatar May 02 '23 17:05 crystalenka

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

obuisard avatar May 05 '23 14:05 obuisard

Agree with Olivier this is a bug as it does not function as expected

sdwjoomla avatar May 05 '23 21:05 sdwjoomla

Crystal @crystalenka can you look at the conflicts? Let us know if you need help with that. Thanks!

obuisard avatar May 05 '23 22:05 obuisard

Thank you! It's on my list and I will check it as soon as I can.

crystalenka avatar May 08 '23 06:05 crystalenka

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.

obuisard avatar May 31 '23 01:05 obuisard

Hi @obuisard , sorry for taking so long. I believe I fixed the conflicts now.

crystalenka avatar Jul 13 '23 09:07 crystalenka

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 :-)

obuisard avatar Jul 13 '23 16:07 obuisard

Log in as a non super user. Edit an article. See console.

38602

Quy avatar Jul 19 '23 18:07 Quy

This pull request has been automatically rebased to 4.4-dev.

HLeithner avatar Sep 30 '23 22:09 HLeithner

This pull request has been automatically rebased to 5.2-dev.

HLeithner avatar Nov 15 '24 13:11 HLeithner

Is this PR still necessary? If yes, can you please fix the conflict?

Hackwar avatar Jan 16 '25 09:01 Hackwar

I've allowed myself to fix the merge conflict.

richard67 avatar Jan 19 '25 10:01 richard67

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.

imrohitkodam avatar Feb 22 '25 06:02 imrohitkodam

I have not tested this item.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38602.

imrohitkodam avatar Feb 22 '25 07:02 imrohitkodam

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.

imrohitkodam avatar Feb 22 '25 07:02 imrohitkodam

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.

Vineet7875 avatar Feb 22 '25 07:02 Vineet7875

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 .

richard67 avatar Feb 22 '25 16:02 richard67

This pull request has been automatically rebased to 5.3-dev.

HLeithner avatar Apr 15 '25 16:04 HLeithner

This pull request has been automatically rebased to 5.4-dev.

HLeithner avatar Oct 15 '25 17:10 HLeithner