The function declaration is

function update_manager_update_form($form, $form_state = array(), $context) {

If the second argument has a default value, then the third argument is supposed to have one, too.


I am not sure of the correct fix: give $context a default value, remove the default value from $form_state, or remove the third argument entirely (and maybe also the default value). AFAICT, $context is not referred to anywhere in the code. I did not see func_get_args() either, but maybe I overlooked something.

Comments

dlu’s picture

dlu commented
Status: Active » Closed (fixed)

This issue duplicates/overlaps with the global move to the new form interface. The patch proposed in #2010246-13: Convert update_manager_install_form, update_manager_update_form, update_manager_update_ready_form to the new form interface. moves and rewrites this function (it becomes UpdateManagerUpdate::buildForm() in lib/Drupal/update/Form/UpdateManagerUpdate.php and there is no longer a $context argument.

The D7 call to update_manager_update_form() is identical to the D8 call that inspired this issue:

function update_manager_update_form($form, $form_state = array(), $context) {

It looks like there are really two issues there. The first is that the second argument, $form_state should be passed by reference &$form_state (although it does not seem like the function actually uses $form_state (or $context for that matter). The second is that, I don't think there is any point in defaulting $form_state since changes to it need to be visible outside of update_manager_update_form().

So, I think the fix for this issues in D7 would be to change the call to:

function update_manager_update_form($form, &$form_state, $context) {

I will create a D7 issues since this doesn't really seem like something that could be backported.

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented

Instead of closing this issue and opening a new one, I would have just changed the version from 8.x to 7.x. Since you already opened the new issue, let's just add a cross reference here: #2051453: syntax error in update_manager_update_form().

dlu’s picture

dlu commented

Thanks, I wasn't sure what the right thing to do with an issue that went two different ways between D7 and D8.

I think the D7 fix ought to be something like this:

function update_manager_update_form($form, &$form_state, $context) {

I'd hoped to verify that this didn't mess something up by running the Update module tests, but they fail against 7.22. I'll post the patch and let the test bot have a whack at it – I'm assuming that I must be doing something wrong with the tests, hopefully the test bot is smarter than me.