Opened 13 years ago
Last modified 2 years ago
#18289 reviewing task (blessed)
Direct link to plugin installation should have admin chrome
Reported by: | nacin | Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | |
Component: | Upgrade/Install | Keywords: | has-patch needs-testing |
Focuses: | Cc: |
Description
We should be able to provide a direct link to the page to install a plugin, based on the plugin's slug. This does it: wp-admin/plugin-install.php?tab=plugin-information&plugin=log-deprecated-notices. However, there's no admin chrome, no real indication which site you're on, and no name of the plugin.
If we're not loading that page inside an iframe request, it needs the admin around it, as well as a heading. Probably new styling too.
This would serve as a replacement for Jaquith's bookmarklet, which broke in 3.2 (frame busting), as well as allow us to integrate a link on extend/plugins for plugin installation. Related, #16923, which is now closed.
Attachments (8)
Change History (29)
#1
@
13 years ago
- Owner set to nacin
- Status changed from new to reviewing
The attached patch adds this functionality. Just add "&wrapped" to the end of the query string.
Couple concerns I have:
- install_plugins_pre_plugin-information is already an action. To get a return value, I had to make it a filter. Didn't want to break existing plugins, so now it's an action AND a filter, with one called instead of the other if iframed. Kinda redundant. Is it worth adding a separate filter for this, or maybe deprecating the action?
- Lots of HTML mixed with code in wp-admin/includes/plugin-install.php. Any chance a templating system could be used here?
#3
@
13 years ago
Surely theres a better way than returning a huge chunk of HTML.
The first thought that came to my mind, was to just have iframe_header() / iframe_footer() include the normal header/footer, however we'd run into some globals issues there.. (Theres a ticket elsewhere for that I believe)
We know the list of actions which can end up in a iframe, and we should be able to detect if it's a iframe'd request (just throwing an extra param into the url for example).
I'll see if i can manage a patch following my thoughts..
#4
@
13 years ago
Patch is to remove thickbox on the install plugin page (first pass). I was working on this when nacin let me know about this ticket. I'm going to try to solve the no-chrome issue along with this since I'm messing with the plugin information page anyway.
#6
@
13 years ago
Updated ticket to show chrome based on no-chrome URL parameter that the AJAX call uses. Then just some logic on where to add_action and if it should exit or not. Played with a couple other ideas of handling it and this was the easiest.
Also, the styling is horribly botched. Made me wonder if we should just redesign this page slightly since it will now show within the admin chrome. Make it fit in a little better? Then could use the same styling within the slide down as well.
#7
@
13 years ago
Also, just thought: since there will now be an in-site plugin information page, we should just link there for no-js, not wp.org (which I changed it to). Updated patch with that.
#8
@
13 years ago
Refreshed patch.
- Improved styling.
- Make it work properly on Multisite
- Some other miscellaneous improvements
#9
@
13 years ago
I'm also repeating a lot of code in that. I think that a lot should be broken up into separate functions but I wanted to get an initial patch up before I started messing with it too much.
#12
@
13 years ago
- Keywords needs-testing added
attachment 18289.5.diff added
- Midnight patch
- Thickbox requests get a iframe'd version, direct access gets a full admin chrome
- Revealed a bug in the JS, looks like the Thickbox wasn't getting it's custom title from the plugins page thanks to the list tables using a different table class
- Had to switch from body_id (which only the iframe handler supports it seems) to a generic body class which is better anyway
- Doesn't do any Ajax magic like the other patches here
- Aims for minimal disruption of existing code
- Seems to work, but needs some extra testing and eyes
Lets not go into doing any UI+UX overhauls/changes here, All install/update functionality needs a facelift, and that needs some thought put into it, I don't think we should go through attempting any of that here
#13
@
13 years ago
- Avert your eyes from the magic that is a static variable and a function hooked to 2 actions in
install_plugin_information_direct()
...
#14
@
13 years ago
$href = add_query_arg('plugin', $_REQUESTplugin?, $href) doesn't look secure especially without escaping in the next line.
#15
@
13 years ago
The patch as-is misses the Plugins widget on the dashboard, and the Importers screen as well.
As for $_REQUEST['plugin']
that should be sanitized with sanitize_key() probably, or just urlencode it instead. (add_query_arg() not encoding/sanitizing things strikes me again..)
I'm not too fussy on this, If someone definitely wants this in 3.3, it'd be appreciated for a patch refresh/update with the above items in mind.
This ticket was mentioned in Slack in #meta by gibrown. View the logs.
3 years ago
#21
@
2 years ago
- Keywords needs-refresh removed
- Milestone set to Future Release
- Owner nacin deleted
Wipes the dust off
Found this one going through a list of tickets missing a milestone. Mulling it over, I think this is still something that would be a nice to have, but definitely a low priority item.
I did some testing with the latest version of WordPress (currently 6.0.1), and wp-admin/plugin-install.php?tab=plugin-information&plugin=query-monitor
still works to pull up the plugin details in the same way as when this was opened (no admin context).
The easiest way to accomplish this without introducing a new UI would be to introduce a new "no menu tab" for displaying one or more specific plugins.
18289.7.diff is a proof of concept patch that allows a comma separated list of plugins to be passed and displayed in the normal plugin view.
Some pros of this approach:
- Uses the same UI.
- Allows only specific plugins to be listed (search does not even guarantee the intended plugin is listed first).
- User can click more details on any of them, and also has an install button to click.
- Allows plugin and theme authors to link to a specific listing of compatible plugins or add-ons.
Some cons:
- The
plugin_information
action returns way more information than needed, and is missing some information for display (plugin icons, for example, as seen in the POC patch). - There's no specific API action to just receive a specific set of plugins.
- Results in one API request per plugin specified. Would be wasteful if many (5, 10, 15+ plugins) are listed.
This could also be adjusted to only support one plugin. It felt reasonable to allow multiple, though.
Patch to provide requested functionality