WordPress.org

Make WordPress Core

Opened 16 months ago

Closed 16 months ago

Last modified 16 months ago

#27639 closed defect (bug) (fixed)

Install Themes: API call to api.wordpress.org fails under IE8/9 because of CORS

Reported by: ocean90 Owned by: nacin
Milestone: 3.9 Priority: high
Severity: normal Version: 3.9
Component: Themes Keywords:
Focuses: Cc:

Description (last modified by ocean90)

Currently the API call to fetch themes fails with the message "No transport".

After some searching I found, that $.support.cors = true; should help. This still ends in an error with "Access is denied".

Attachments (3)

27581.patch (4.6 KB) - added by adamsilverstein 16 months ago.
use wp-ajax and json api for apiCall
27639.patch (4.6 KB) - added by adamsilverstein 16 months ago.
use wp-ajax and json api for apiCall
27055.diff (4.0 KB) - added by jorbin 16 months ago.

Download all attachments as: .zip

Change History (17)

comment:1 @ircbot16 months ago

This ticket was mentioned in IRC in #wordpress-dev by ocean90. View the logs.

comment:2 @nacin16 months ago

Okay, if the browser doesn't support CORS, we can proxy these through PHP, as we had been doing before.

comment:3 @ocean9016 months ago

  • Description modified (diff)

comment:5 @nacin16 months ago

As indicated in #27581 we can also go the JSONP route. The API now supports the following:

  • ?callback=
  • tag: to be comma-separated rather than an array, to save on length
  • fields: to be comma-separated rather than an array, to save on length (for this, it assumes all are true)

I'd suggest that everything be consolidated in apiCall; other areas shouldn't need to know tags should be imploded; it makes it harder to manipulate.

@adamsilverstein16 months ago

use wp-ajax and json api for apiCall

@adamsilverstein16 months ago

use wp-ajax and json api for apiCall

comment:6 @adamsilverstein16 months ago

  • Keywords has-patch dev-feedback added; needs-patch removed

27639.patch is based on johnbillion's patch - added transient caching; tested in ie8, resolves the ticket issue;

comment:7 follow-up: @nacin16 months ago

I don't think we need transient caching for this. We're dealing with a fairly arbitrary features matrix and arbitrary searching... It would explode the database without much benefit. It's unlikely to see much crossover with someone else's results; meanwhile, the JS maintains a local cache for that particular person.

I'd still like to see an attempt at JSONP and avoiding the proxy.

That said, if we were to proxy, then we should use themes_api() and bring in the filters we lost in WP_Theme_Install_List_Table::prepare_items().

comment:8 @nacin16 months ago

Also: A nonce isn't needed for a fetching action.

comment:9 in reply to: ↑ 7 @adamsilverstein16 months ago

  • Keywords needs-patch added; has-patch dev-feedback removed

Replying to nacin:

I don't think we need transient caching for this. We're dealing with a fairly arbitrary features matrix and arbitrary searching... It would explode the database without much benefit. It's unlikely to see much crossover with someone else's results; meanwhile, the JS maintains a local cache for that particular person.

I'd still like to see an attempt at JSONP and avoiding the proxy.

That said, if we were to proxy, then we should use themes_api() and bring in the filters we lost in WP_Theme_Install_List_Table::prepare_items().

Thanks for the feedback, good point about the transient caching. I will give the JSONP approach a shot.

@jorbin16 months ago

comment:10 @jorbin16 months ago

I missed this being opened and instead posted to the original THX ticket. My proposed solution is not very different from Adams. I've posted it here though for posterity.

comment:11 follow-up: @nacin16 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 27961:

Theme Installer: Use JSONP for api.wordpress.org requests.

fixes #27639.

comment:12 in reply to: ↑ 11 @adamsilverstein16 months ago

  • Keywords needs-patch removed

Replying to nacin:

In 27961:

Theme Installer: Use JSONP for api.wordpress.org requests.

fixes #27639.

Nicely done sir!

comment:13 @ircbot16 months ago

This ticket was mentioned in IRC in #wordpress-dev by ocean90. View the logs.

comment:14 @nacin16 months ago

In 28126:

Theme Installer: Revert to proxying through PHP for WordPress.org API requests.

This is to ensure we have valid installation nonces, though we've run into this as a problem previously (see #27639, #27581, #27055).

A tad slower, but we gained speed in 3.9 by simplifying the request made to the API.

props ocean90.
fixes #27798.

Note: See TracTickets for help on using tickets.