User tests: Successful: Unsuccessful:
Pull Request for Issue #23696
Related and already merged fix: #23501
Install a J4 nightly build of today (Monday, 28 January 2019) or update an older J4 with that package
Go to menu items manager in back-end.
Open Home
menu item (type "Articles > Featured Articles")
Save it.
Error like described in #23696 (comment)
Apply patch
Try again to save the menu item.
Should be successful.
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_menus |
I'm also concerned because I don't understand this necessary hardcoding of field names inside models when I read the introduction of pr #12414 of @Hackwar concerning "hardcoded behavior" and "hardcoded filters" . At the moment this pr here and #23501 are just bad "outsourcing of occuring new issues" somehow.
On the other hand the line $form->removeField('show_associations', 'params');
provides the fields group, too. Shouldn't that be sufficient to avoid field name conflicts? I don't know.
I don't know how similiar behavior can be implemented in the concerning field classes directly or if that's possible anyhow.
I have tested this item
@infograf768 you have effectively already hardcoded the usage of that parameter name. If you have issues with the Form changes, then maybe make sure that the associations field doesn't behave in the way it does.
@Hackwar
I do not understand what you are saying.
After #12414 , @joomdonation had to do #23501
I had nothing to do with these changes.
It is not an association field per se as in the Associations Tab.
show_associations
just lets show for an article in frontend an icon or lang tag of the associated article in the block.
It looks partly broken for now in 4.0 but this is what it looks like in 3.x
I have tested this item
- After installing nightly build (update package) from 28th
Please do NOT set this as RTC until @Hackwar and @joomdonation give more precise informations on a better way to solve this.
We have two options here:
Check in all of our core components to see which one used that field and do the same fix like I did on this PR #23501
Or restore the original code which @Hackwar implemented in his PR. Basically, the command below before this line https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Form/Form.php#L1202
if (!$fieldObj instanceof FormField)
{
continue;
}
To me, #1 would be better.
I don't work much with multilingual, so I am unsure. I will try to look at this issue later today to see what's the actual reason of the error and hopefully can come up with a proper fix.
Or restore the original code which @Hackwar implemented in his PR. Basically, the command below before this line https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Form/Form.php#L1202
Tested this and it works fine.
It's certainly wrong to have this code in com_menus. It's a com_content specific parameter.
If you want this to be conditional, you will have to try with a custom formfield for com_content.
@joomdonation Unless this change is documented somewhere, option 2 is the proper fix. The form should not fail when FormField::setup()
returns false
.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-01-30 11:15:07 |
Closed_By | ⇒ | ReLater | |
Labels |
Added:
?
|
I am concerned by one aspect:
If a 3pd uses the same field name for other stuff than articles, it would remove the field.