Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI doesn't obey custom WP_CONTENT_DIR in Directory/Expiration Time #206

Closed
raamdev opened this issue Jun 9, 2014 · 11 comments
Closed

UI doesn't obey custom WP_CONTENT_DIR in Directory/Expiration Time #206

raamdev opened this issue Jun 9, 2014 · 11 comments

Comments

@raamdev
Copy link
Contributor

raamdev commented Jun 9, 2014

When a custom WP_CONTENT_DIR is set, the Directory/Expiration Time --> Base Cache Directory does not show the custom path, but rather uses the default.

Reported here: http://wordpress.org/support/topic/base-cache-directory?replies=1#post-5662043

@raamdev raamdev added this to the Next Release milestone Jun 9, 2014
@jaswrks
Copy link

jaswrks commented Jun 10, 2014

Hmm. I don't think that it should. The configured cache directory is always relative to ABSPATH, not to WP_CONTENT_DIR. When we updated this awhile back it was to get the default option value right, not to correct the path internally. QC still uses ABSPATH.[configured cache directory] to formulate the cache directory location. This is actually more flexible, because it allows you to put it inside your WP_CONTENT_DIR (default behavior); or somewhere else if you prefer. So long as it's inside ABSPATH.

@raamdev
Copy link
Contributor Author

raamdev commented Jun 10, 2014

See below.

2 similar comments
@raamdev
Copy link
Contributor Author

raamdev commented Jun 10, 2014

See below.

@raamdev
Copy link
Contributor Author

raamdev commented Jun 10, 2014

See below.

@raamdev
Copy link
Contributor Author

raamdev commented Jun 10, 2014

False alarm

Sorry, everything is working as expected, including the path shown in the UI. (see next comment)

Quick Cache writes cache files to the custom WP_CONTENT_DIR path AND it shows that path in Directory/Expiration Time --> Base Cache Directory. (see next comment)

The problem was that the previous configuration was being used. I needed to press "Restore" in Quick Cache to reset its settings. (I thought I had done that before testing, but I guess not!)

@raamdev raamdev closed this as completed Jun 10, 2014
@raamdev raamdev removed this from the Next Release milestone Jun 10, 2014
@raamdev raamdev removed bug labels Jun 10, 2014
@raamdev
Copy link
Contributor Author

raamdev commented Jun 10, 2014

Actually, maybe there's still an issue here...

@jaswsinc writes...

The configured cache directory is always relative to ABSPATH, not to WP_CONTENT_DIR

But what if WP_CONTENT_DIR is set to a path outside the ABSPATH?

// ABSPATH = /var/www/qcpro.dev/wordpress/
define('WP_CONTENT_DIR', '/var/www/qcpro.dev/data/wp-content');
define('WP_CONTENT_URL','http://qcpro.dev/data/wp-content');

Then, the Quick Cache UI would reflect the wrong path, because it shows:

ABSPATH . WP_CONTENT_DIR . "cache/quick-cache/cache/"

That said, the cache files would still be written to the correct place because this line in quick-cache.inc.php would simply return the original value of WP_CONTENT_DIR (i.e., it wouldn't find ABSPATH in WP_CONTENT_DIR):

$wp_content_dir_relative = // Considers custom `WP_CONTENT_DIR` locations.
    trim(str_replace(ABSPATH, '', WP_CONTENT_DIR), '\\/'." \t\n\r\0\x0B");

@raamdev raamdev reopened this Jun 10, 2014
@raamdev raamdev added the bug label Jun 10, 2014
@raamdev raamdev added this to the Next Release milestone Jun 10, 2014
@jaswrks
Copy link

jaswrks commented Jun 10, 2014

This line and this method will always force whatever is configured to be referenced under ABSPATH.

If WP_CONTENT_DIR is configured to something outside of ABSPATH, the QC base_dir will end up being something like ABSPATH./directory/outside/abspath/.[configured base directory]

I think that's as it should be. No matter what, we need the directory to be within ABSPATH so that it's possible to build a URL to reference files inside the directory; i.e. site_url() needs to work as expected here.

@jaswrks
Copy link

jaswrks commented Jun 10, 2014

I suppose if we use content_url() instead of site_url() then it would be OK. If we do that, we will also need to check the HTML Compressor to be sure it remains compatible also.

@raamdev
Copy link
Contributor Author

raamdev commented Jun 10, 2014

This line and this method will always force whatever is configured to be referenced under ABSPATH.

In that case, I feel that #95 needs to be reopened, as technically Quick Cache does not fully support a custom WP_CONTENT_DIR and WP_CONTENT_URL.

WordPress specifically mentions that WP_CONTENT_DIR can be changed to a "full local path" (the example used is relative to the location of wp-config.php, but I think that's besides the point): http://codex.wordpress.org/Editing_wp-config.php#Moving_wp-content_folder

If a site owner can change WP_CONTENT_DIR to a path outside ABSPATH, then I think we should assume that will be the case. Do you agree?

The docs also say that if you move your WP_CONTENT_DIR, you must also define WP_CONTENT_URL, so that files in that custom directory are web-accessible. That means we should be able to rely on content_url() instead of site_url() as you mentioned.

@jaswrks
Copy link

jaswrks commented Jun 10, 2014

Yep, I agree. I don't see this being a major issue either. QC currently doesn't use site_url() much at all anyway, and where it does use it, it's warranted (i.e. for the sitemap location).

I also checked the source code for the HTML Compressor, and it uses both WP_CONTENT_DIR and WP_CONTENT_URL when they are available; so we are already good there too.

I think all that needs to happen here, is that we need to remove wp-content from the base_dir in the QC options array; and replace existing occurrences of ABSPATH with WP_CONTENT_DIR as the new root directory from which to nest everything else into. The method abspath_to() should get renamed to wp_content_dir_to() maybe.

@raamdev
Copy link
Contributor Author

raamdev commented Jun 10, 2014

Got it. Thanks! I'll reopen #95 and track this issue specifically for updating the UI.

jaswrks pushed a commit to wpsharks/comet-cache-pro that referenced this issue Jun 13, 2014
@raamdev raamdev changed the title Custom WP_CONTENT_DIR not obeyed in Directory/Expiration Time UI UI doesn't obey custom WP_CONTENT_DIR in Directory/Expiration Time Jun 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants