Easy-digital-downloads: WP core file editor not working with EDD activated

Created on 19 Dec 2017  路  16Comments  路  Source: easydigitaldownloads/easy-digital-downloads

This is a strange one. It took a minute to be able to replicate but I can do it consistently.

First, make sure you're on the latest version of WordPress. At least 4.9, which introduced some changes to the Plugin/Theme file editors.

There's a somewhat common issue that people are reporting here: https://wordpress.org/support/topic/cant-edit-main-theme-php-files-after-upgrading-to-4-9/ I haven't read this entire thread yet... working on it. But the same error seems to occur when EDD is activated. That is...

  1. Leave EDD activated
  2. Go to Appearance -> Editor and select a PHP file from your theme. Any PHP file should be fine. Edit the file... any type of edit. The page will hang for a few minutes before displaying an error: https://cl.ly/2O1x2D2m1b3e
  3. Deactivate EDD (EDD's extensions have nothing to do with this... they can all be deactivated in step 1)
  4. Repeat step 2. It should work fine.
type-bug

All 16 comments

Update: it seems you can edit a theme that is not active. But you cannot edit a theme that is active.

What's weird, and why I opened an issue here, is because this behavior is toggled by EDD being activated or deactivated. Doesn't matter what theme files you're trying to edit. If it's the active theme, and EDD is activated, the error occurs.

Looks like we have a customer here seeing this.

I've tested this all the way back to ED 2.7.10 and the problem still occurs.

Looks like this happens on sites that use PHP sessions.

Adding define( 'EDD_USE_PHP_SESSIONS', false ); resolves it.

I believe I've fixed this by setting up our session handler to only start in is_admin() when an ajax request is being processed. If it's not an ajax request, we set $start_session to false.

I'm not sure why this issue only appears on some sites and not all sites that use PHP sessions, but on the customer's site I was testing with, this did resolve it.

Try this instead:

if( false !== strpos( $uri, 'wp_scrape_key' ) ) {
    $start_session = false;
}

Added.

That has fixed it for the customer's site I was testing on.

I was not able to replicate this on my end. @pippinsplugins @mindctrl @SDavisMedia are we confident with this current resolution? Relying on people who could replicate to verify. It all looks sane to me from a code perspective.

I was able to replicate and verify that the proposed patch fixes it.

Sounds good, merging then. thanks @mindctrl

Is the session stuff only meant to be used on the front-end? I'm currently using some session storage in the admin area and ran across the issue of it not actually being stored.

Instead of only allowing sessions for admin-ajax, wouldn't it be better to stop it from initializing for file-editor requests?

In general, yes, sessions should only be used on the front end and in admin-ajax. If you need session data in other parts of the admin, you will need to overwrite the boolean flag with the edd_start_session filter.

Is there any reasoning behind that? - Just for my knowledge > as I'm not aware of any reason why a session wouldn't be able to run on the admin / the EDD_Session class is initialized already.

I think we might need to look at reverting part of this change. Specifically the part that prevents the session from loading in wp-admin because it is breaking file uploads in FES.

However, we can leave @mindctrl's recommended portion of the change made here regarding the wp_scrape_key. In my tests it resolves the originally-reported issue on its own (editing php files in wp-admin's editor).

To clarify, I tested it with this removed and it allowed me to edit a php file in the wp code editor:

if( is_admin() && false === strpos( $uri, 'wp-admin/admin-ajax.php' ) ) {
    // We do not want to start sessions in the admin unless we're processing an ajax request
    $start_session = false;
}

So the only required portion to fix this issue is this:

if( false !== strpos( $uri, 'wp_scrape_key' ) ) {
    // Starting sessions while saving the file editor can break the save process, so don't start
    $start_session = false;
}

This will resolve the FES problem:
https://github.com/easydigitaldownloads/edd-fes/issues/1408

@mintplugins could you please go ahead and open a separate issue for this and submit a PR? We can put it in the next point release.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

SDavisMedia picture SDavisMedia  路  3Comments

DevinWalker picture DevinWalker  路  6Comments

Ismail-elkorchi picture Ismail-elkorchi  路  3Comments

amdrew picture amdrew  路  5Comments

JeroenSormani picture JeroenSormani  路  5Comments