themes_api_result filter should use original arguments

[Corphi]

The Themes API currently behaves differently whether it was overridden or it used its native method of querying theme information.

If the API is overridden by a custom implementation, the $args argument to the filter is - as documented - the original $args object that was passed to the API.

If the API uses the native method, the $args variable gets overridden by an array of arguments to a HTTP query. This behavior is neither documented nor intuitive.

The attached patch separates these two concerns and makes the HTTP query arguments available as a further parameter to the filter. But to me, they only seem useful for debugging (where WP_Http seems the right place to hook into) and should probably be dropped entirely. I created a patch for this scenario, too.

[Corphi]

Separate query args, use as additional filter parameter.

[Corphi]

Separate query args, don’t hand HTTP args to the filter.

[obenland]

I agree, variable names could be a little more verbose to be less confusing.
http-args-dropped.patch​ seems like a good way to do that.

It looks like it was introduced in r25956, breaking backwards compatibility for the themes_api_result filter.
Fixing it would restore previous behavior, but would break compatibility with implementations since then. :(

[Corphi]

Problem persists through version 4.1.
By ​definition in the glossary, this is a bug (unexpected result).

[SergeyBiryukov]

This applies to the plugins_api_result​ filter as well.

These filters were introduced in 2.7 and 2.8, so [25956] was relatively not too long ago.

http-args-dropped.patch​ seems like the way to go, perhaps with a post on make/core.

I'll do a quick search through the plugin repo to see if there are any affected plugins.

[SergeyBiryukov]

Results from the repository:

awesome-flickr-gallery-plugin/afg_update.php
caldera-forms/classes/selldock_updater.php
crm-by-tpc/TPC/Helper/AutoUpdate.php
free-guest-post/ghost_writer.php
kento-pricing-table-free/kento-pricing-table-free.php
layout-engine/themes.php
myblogu/myblogu.php
orbisius-member-only-downloads-for-s2member/orbisius-s2member-only-downloads.php
plugin-bundles/includes/bundles_overview.php
plugin-download-display/plugin-download-display.php
plugin-promoter/plugin-promoter.php
plugintel/plugintel.php
pricingtable/pricingtable.php
sendpress/classes/class-sendpress-pro-manager.php
social-deux/admin/includes/wp-updates-plugin.php
wp-autoloader/lib/WPExtend/BasicWPFilters.php
wp-plugin-installer/wp-plugin-installer.php
wpzing/includes/core.php

​WP Plugin Installer would be fixed by this change, others appear to be unaffected.

Searching through themes now, just in case.

[obenland]

Nice! We could add those in the post and talk about it breaking that plugin.

[SergeyBiryukov]

It actually fixes that plugin :) (Assuming it still works otherwise, haven't tested.)

No matches for (plugins|themes)_api_result in the theme repository.

[Ipstenu]

Thanks, @dd32, for alerting. Don't worry about the broken plugins. We'll handle it :)

[dd32]

Replying to Ipstenu:

Thanks, @dd32, for alerting. Don't worry about the broken plugins. We'll handle it :)

FYI for the ticket; Half of these plugins were including a custom update handler - which is a big no-no for the plugins directory. The other half only had http functions, or included update functionality for other things (a few offer extra repo's for themes, optional extras for install, or premium upgrade paths) which were fine.

[SergeyBiryukov]

In 31363:

Avoid inadvertent stomping of the original $args parameter passed to plugins_api_result and themes_api_result filters in plugins_api() and themes_api(), respectively.

Fixes a regression introduced in [25956].

props Corphi, obenland.
fixes #29079.