Needs work
Project:
Drupal core
Version:
main
Component:
forms system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
10 Mar 2026 at 17:09 UTC
Updated:
22 May 2026 at 13:35 UTC
Jump to comment: Most recent
Show commands
changes
Comments
Comment #2
sourav_paulWorking on it..
Comment #4
longwaveThanks - in most cases we shouldn't need the attribute at all, because the service can be inferred from the object/interface type that is required. Also, coding standards checks are failing.
Comment #5
sourav_paulThanks @longwave for the clarity. I've implemented those changes as per suggestion. kindly let me know if there is anything need to change.
Comment #6
sourav_paulComment #7
longwaveSome test failures to sort out.
Also we need to decide for which forms we want to provide BC in the constructor, where before e.g. we passed an entity storage handler and now we pass the entity type manager instead. I imagine most downstream changes are via form alter hooks instead of actually extending? Not sure.
Comment #8
sourav_paulThanks for the feedback. I investigated the failing jobs and identified the root cause: removing all #[Autowire] attributes broke autowiring for ConfigurableLanguageManagerInterface in language forms.
CI failure source was: AutowiringFailedException for NegotiationConfigureForm::__construct(), which then caused the widespread “Save settings/Add language button not found” failures.
I restored explicit #[Autowire(service: 'language_manager')] only for the forms that require it (LanguageFormBase, NegotiationBrowserForm, NegotiationConfigureForm).
On constructor BC: the storage→entity type manager signature changes are in @internal forms (NodeRevisionDeleteForm, NodeRevisionRevertForm, NodeRevisionRevertTranslationForm, NegotiationConfigureForm). I’ve left them as-is for now, but I can add BC constructor handling if we want extra compatibility even for internal classes.
Comment #10
sourav_paul@sivaji_ganesh_jojodae Thanks for rebasing the branch, But this is not a correct way to work on any issue while someone is already working on it (atleast you should wait 3days).
Comment #11
longwaveLet's minimise the changes required here where we can - there is no need to reformat most of the constructors.
Comment #12
sourav_paulThanks @Dave for your guidance, I've removed all constructor white space formatting.
Feel free to know, if there anything you need to fix.
Comment #13
smustgrave commentedNot to expand scope too much but think since we are touching these files we should do constructor promotion while we are at. If not lets least drop the docs since that's not the default anymore.
Comment #14
sivaji_ganesh_jojodae commentedI am working on the constructor property promotion.
Comment #15
sivaji_ganesh_jojodae commentedI think necessary changes for constructor property promotion are committed.
Comment #16
sivaji_ganesh_jojodae commentedComment #17
smustgrave commentedThanks other part of my comment was also dropping the docs on the construct now
Comment #18
sivaji_ganesh_jojodae commentedRemoved the constructor PHPDoc blocks. Also updated the issue summary to include the scope expansions mentioned in the comments.
Comment #19
smustgrave commentedLooks much cleaner now, no further feedback from me.
Comment #20
longwaveOpened #3586556: Add a service alias for ConfigurableLanguageManagerInterface as a followup which would clean up the three instances of
#[Autowire(service: 'language_manager')].Comment #21
longwaveAdded some comments.
Not sure any of the config forms should use property promotion for the typed config manager, as they pass it to the parent, ie.
should maybe be
Opened #3586565: Consider using #[Required] to inject dependencies into ConfigFormBase to investigate if we can remove this entirely.
Comment #22
sivaji_ganesh_jojodae commented@longwave, thanks for catching the
TypedConfigManagerInterfaceproperty -- that’s been fixed in my latest update. I’ve applied a similar fix to the properties inNodeRevisionRevertTranslationForm.phpas well.There are a few other open comments; could you clarify what changes are expected there?
Comment #23
dcam commentedI added a few comments to the MR, including two items that need to be addressed.
If I counted correctly there are currently four classes that break backward compatibility. Forms are considered internal, but committers may require that backward compatibility be added to them anyway. That said, I'm willing to RTBC the MR and send it up for committer review if the other issues that I flagged are addressed.