Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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
Comment | File | Size | Author |
---|---|---|---|
#12 | 613654.patch | 2.56 KB | RobLoach |
#11 | 613654.patch | 2.59 KB | RobLoach |
#10 | 613654-dialog.patch | 2.16 KB | RobLoach |
#2 | 613654-2.update_manager_js.patch | 1.32 KB | dww |
#2 | 613654-2.update_manager_js.no_js.png | 66.72 KB | dww |
Comments
Comment #1
Dave ReidI'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.
Comment #2
dwwOh, 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).
Comment #3
dwwComment #4
Dries CreditAttribution: Dries commentedDo we want to start introducing popups at this stage in the game? To me, this feels more like a D8 improvement.
Comment #5
dww@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.
Comment #6
dwwFor the history-conscious, this pop-up stuff was introduced at #395474-218: Plugin Manager in Core: Part 2 (integration with update status)...
Comment #7
sun+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.
Comment #8
dwwBy "elsewhere", sun means: #87994: Quit clobbering people's work when they click the filter tips link ;)
Comment #9
dwwActually, 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...
Comment #10
RobLoachThis 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.
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?
Comment #11
RobLoachAdded 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.
Comment #12
RobLoachWhoops, this fixes it.
Comment #13
RobLoachHeh, the following works too: http://drupalbin.com/12117 , but doesn't look as nice.
Comment #14
dwwSuper 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...
Comment #15
lyricnz CreditAttribution: lyricnz commentedupdate.manager.js still 404. How can this be critical+postponed (since October)?
Comment #16
dwwIt'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.
Comment #17
quiptime CreditAttribution: quiptime commentedWhere is the file "update.manager.js"?
The error
still exists.
Comment #18
mfbAlso 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()
Comment #19
marvil07 CreditAttribution: marvil07 commentedI 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
Comment #20
dwwYeah, 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...
Comment #21
webchickThere 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.
Comment #22
David_Rothstein CreditAttribution: David_Rothstein commentedIt 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.
Comment #23
lyricnz CreditAttribution: lyricnz commentedD7 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?
Comment #24
scronide CreditAttribution: scronide commentedThere 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.