In update.manager.inc, update_manager_update_form() adds a update.manager.js to the form, but modules/update/update.manager.js is missing from CVS.

CVS annotate says that line was committed in #538660: Move update manager upgrade process into new authorize.php file (and make it actually work)

This might be the reason that I'm unable to get the admin/config/modules/install page to work. But probably not, since I'm seeing “Warning: Parameter 1 to update_manager_install_form() expected to be a reference, value given”. :-p

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

Priority: Normal » Critical

I'm beginning to think that there is no update.manager.js. I can't find it in any of the patches in that issue. dww is usually very meticulous with new files.

dww’s picture

Assigned: Unassigned » dww
Status: Active » Needs review
Issue tags: +Update manager
FileSize
63.89 KB
50.5 KB
66.72 KB
1.32 KB

Oh, bah. Sorry about that. Seems to have gotten lost at some point in the other issue.

What it's trying to do is a little silly, but it's probably a good UI enhancement for edge cases. If you're running a releases from a branch that's no longer the recommended branch, and update manager is about to jump you from 7.x-1.? to 7.x-2.? or something, it prints you a big nasty warning about it. The JS hides the warnings from the table until you actually check the checkbox for that release, and then dumps you the warning as a popup.

The JS doesn't completely work yet (see all the @todo's I added when I was cleaning all this up and renaming files), but it's slightly better than it was when this lived directly in update.js, mixed in with the code that became misc/authorize.js...

Anyway, here's a patch that adds the file, and some screenshots of seeing it in action. (If you install http://drupal.org/node/521012 aka http://ftp.drupal.org/files/projects/update_test_module-7.x-1.0.tar.gz and edit the .info file to change the version to 7.x-0.1, you could see this in action. I should probably make a 7.x-0.1 release of update_test_module so people can mess with this directly).

dww’s picture

Component: update system » update.module
Dries’s picture

Do we want to start introducing popups at this stage in the game? To me, this feels more like a D8 improvement.

dww’s picture

@Dries: this was part of the original patch, and (I think) discussed in one of the earlier monster UI issues, and everyone was cool about it. This issue is just about the fact that this file was left out of the original commit... I think the "no_js" screenie is pretty terrible, especially if there are more than 1 project which would jump major versions.

dww’s picture

For the history-conscious, this pop-up stuff was introduced at #395474-218: Plugin Manager in Core: Part 2 (integration with update status)...

sun’s picture

Issue tags: +D7 UX freeze

+1 for the user interface idea behind this patch. We want to leverage the ['#attached']['ui'] introduced elsewhere though.

And, yeah, @Dries, we want to *use* jQuery UI. Otherwise, there would have been little point in adding it to Drupal core in the first place.

This is a nice UX freeze issue, so tagging accordingly.

dww’s picture

dww’s picture

Status: Needs review » Postponed

Actually, looks like the fate of this issue is blocked on #87994: Quit clobbering people's work when they click the filter tips link for now...

RobLoach’s picture

FileSize
2.16 KB

This replicates what dww put up at #2 using the current state of the API at #87994: Quit clobbering people's work when they click the filter tips link.

      $recommended_version .= '<div title="Major upgrade warning" class="update-major-version-warning" id="warning-' . $project['name'] . '">' . t('This update is a major version update which means that it may not be backwards compatible with your currently running version.  It is recommended that you read the release notes and proceed at your own risk.') . '</div>';
      // Create a dialog box for the warning.
      drupal_add_ui(array(
        'selector' => '#warning-' . $project['name'],
        'library' => 'ui.dialog',
        'arguments' => array(
          array(
            'autoOpen' => FALSE,
          ),
        ),
      ));
      // Display the dialog box only when the associated check box is enabled.
      drupal_add_ui(array(
        'selector' => '#warning-' . $project['name'],
        'library' => 'ui.dialog',
        'arguments' => array('open'),
        'bound_element' => ".form-checkbox[value='{$project['name']}']",
        'event' => 'change',
        'is' => ':checked',
      ));

Ran into a problem with the buttons as I'm not quite sure how to pass a JavaScript function pointer through PHP. Look at jQuery UI Dialog's documentation for the options buttons, you'll see it expects a JavaScript function. How could we pass that through in PHP?

RobLoach’s picture

FileSize
2.59 KB

Added a "functions" associative array that processes the arguments into function callbacks. This allows both an Okay button, which will just close the dialog, and a Cancel button, which will uncheck the box and close the dialog. This depends on the latest patch in #87994: Quit clobbering people's work when they click the filter tips link.

      $recommended_version .= '<div title="Major upgrade warning" class="update-major-version-warning" id="warning-' . $project['name'] . '">' . t('This update is a major version update which means that it may not be backwards compatible with your currently running version.  It is recommended that you read the release notes and proceed at your own risk.') . '</div>';
      // Create a dialog box for the warning.
      drupal_add_ui(array(
        'selector' => '#warning-' . $project['name'],
        'library' => 'ui.dialog',
        'arguments' => array(
          array(
            'autoOpen' => FALSE,
            'buttons' => array(
              'Okay' => 'closedialog',
              'Cancel' => 'canceldialog',
            ),
            'dialogClass' => 'warning-dialog',
          ),
        ),
        'functions' => array(
          'closedialog' => '$(".warning-dialog").dialog("close");',
          'canceldialog' => "$('.form-checkbox[value=\"{$project['name']}\"]').attr('checked', false); $('.warning-dialog').dialog('close');",
        ),
      ));
      // Display the dialog box only when the associated check box is enabled.
      drupal_add_ui(array(
        'selector' => '#warning-' . $project['name'],
        'library' => 'ui.dialog',
        'arguments' => array('open'),
        'bound_element' => ".form-checkbox[value='{$project['name']}']",
        'event' => 'change',
        'is' => ':checked',
      ));
RobLoach’s picture

FileSize
2.56 KB

Whoops, this fixes it.

RobLoach’s picture

Heh, the following works too: http://drupalbin.com/12117 , but doesn't look as nice.

dww’s picture

Assigned: dww » Unassigned

Super cool. The "cancel" box even works. ;)

However, this doesn't play nicely with the tableselect. :( That's probably asking way too much. But, if you tableselect, that doesn't trigger the popup. There's also an edge case with the cancel button... say you have 2 projects to update. You click one. You click the other and that triggers the popup and sets the "everything selected" checkbox in the table header. You click "cancel" in the dialog, and it unselects the row, but it leaves the "everything's selected" checkbox checked...

I'm actually going to unassign myself for now, since it seems RobLoach is doing more of the heavy lifting on this issue at this point, and I have plenty of other issues to attend to...

lyricnz’s picture

update.manager.js still 404. How can this be critical+postponed (since October)?

dww’s picture

It's postponed b/c of #87994: Quit clobbering people's work when they click the filter tips link. :( The drop is always moving, but sometimes very slowly. ;) If you want to help here, please help test and get #87994 committed. Then we can finally fix this.

quiptime’s picture

Where is the file "update.manager.js"?

The error

page not found, modules/update/update.manager.js

still exists.

mfb’s picture

Also not found: misc/ui/ui.core.js misc/ui/ui.dialog.js misc/ui/ui.all.css misc/ui/ui.dialog.css Instead of adding these directly it should use drupal_add_library()

marvil07’s picture

Version: 7.x-dev » 8.x-dev

I am moving this to 8.x since the related issue #87994: Quit clobbering people's work when they click the filter tips link is targeted to 8.x

dww’s picture

Version: 8.x-dev » 7.x-dev
Status: Postponed » Active

Yeah, except we can't ship D7 with things in the current situation. If filter tips modal popup was moved to D8, then we have to either reinvent the modal wheel for this, or come up with a different UI for this page before D7 ships. But leaving code to load a .js file that was left out of the original commit for the Update manager is not okay...

webchick’s picture

Priority: Critical » Normal

There needs to be some sort of justification here for this being marked critical. As far as I can see from testing it 3 seconds ago, update manager works perfectly fine as-is.

A patch to remove the call to update.manager.js that we're not using is a one-liner patch, which we should do so that Drupal doesn't barf out errors, but this shouldn't hold up release.

David_Rothstein’s picture

It was a shame to see #87994: Quit clobbering people's work when they click the filter tips link go (given how many times it had been marked RTBC), but I think one of the issues that came up at the end there was accessibility concerns with the dialog popup, so presumably those would apply here too.

lyricnz’s picture

Version: 7.x-dev » 8.x-dev
Status: Active » Needs work

D7 is fixed since #751752: Update directly adds ui files and is broken by 1.8 update. Moving issue back to D8. Feel free to close, if there's an outstanding issue that implements the update-manager JS elsewhere?

scronide’s picture

Status: Needs work » Closed (cannot reproduce)

There is no reference to update.manager.js in D8 and it looks like the pop-up dialog idea that caused it was ditched long ago. Marking closed.