Skip to content
This repository

Problem with CMB_META_BOX_URL creation method #31

Closed
smeric opened this Issue October 12, 2011 · 33 comments

10 participants

Sébastien Méric Norcross Justin Sternberg Bill Erickson Jared Atchison Gary Jones David Higginbotham Alex Mills Wycks sampc
Sébastien Méric

hi,

i went into an issue withe the CMB_META_BOX_URL constant creation method.
i'm working localy on wamp with wp3.2.1

this is what i've got :

echo WP_CONTENT_DIR;
C:\Program Files (x86)\EasyPHP-5.3.6.0\www\wordpress\wordpress3.2.1__/wp-content__

as you can see, the WP_CONTENT_DIR constant has a slash before the last directory instead of a back slash.
i don't know why, so maybe that comes from me and my easyphp install, or from windows... if you think so, please tell me.

so i had to replace your :

define( 'CMB_META_BOX_URL', trailingslashit( str_replace( WP_CONTENT_DIR, WP_CONTENT_URL, dirname(FILE) ) ) );

with this one :

define( 'CMB_META_BOX_URL', trailingslashit( str_replace( str_replace( '\', '/', WP_CONTENT_DIR ), str_replace( '\', '/', WP_CONTENT_URL ), str_replace( '\', '/', dirname(FILE) ) ) ) );

that way i'm sure to get slashs everywhere !

not sure that is the best way to do it, so, please, let me know what you think :)

thank you for this very usefull lib ! i'm just starting tests with it, but seems promising :)

seb.

Bill Erickson
Collaborator

Does anyone know if there's a better way to do this? I don't have windows so can't test, but it seems like there must be a cleaner way than all those str_replace()'s

Jared Atchison
Collaborator

This is a known bug that has been around since the introduction of CMB_META_BOX_URL.

Most of us are Mac users so it is hard to get this bug squashed since it only happens running XAMP/WAMP on a Windows machine. I think the best solution would probably be to look how other plugins are handling similar situations.

Props to your fix above, however we are going to hold out just a little longer and see if a 'cleaner' fix pops up :)

Bill Erickson
Collaborator
Jared Atchison
Collaborator

Well the problem is that we can't use WP_PLUGIN_DIR, plugin_dir_url(), etc :\

Bill Erickson
Collaborator

Ah okay, well what about this: http://code.google.com/p/wolfcms/issues/detail?id=235

I think the issue is the trailingslashit() function which adds a /, not a . Can we replace that with DIRECTORY_SEPARATOR constant?

(Since I don't have a PC I can't test these things)

Gary Jones

Since WP runs on IIS just fine, I suspect that this is something that falls within the WP remit, and if it was a problem, would have been fixed by now.

@smeric - although it looks bad, does the incorrect slash actually stop the resources from loading correctly?

Sébastien Méric

@GaryJones : yes because with this incorect slash, the str_replace function doesn't replace the local path with the url and the jquery.cmbScripts.js, the style.css and the ico-delete.png are not included in the administration tool...

Sébastien Méric

the dirname(__file__) returns :
C:\Program Files (x86)\EasyPHP-5.3.6.0\www\wordpress\parnassus\wp-content\...
with a back slash before \wp-content, and the WP_CONTENT_DIR constant returns :
C:\Program Files (x86)\EasyPHP-5.3.6.0\www\wordpress\parnassus/wp-content
with a normal slash before /wp-content.

so the 2 strings are not matching and str_replace doesn't work properly...

Gary Jones

The "incorrect" slash is added as part of the ABSPATH definition.

Just for kicks, try replacing the '/' in the definition of ABSPATH in wp-load.php WP core file with DIRECTORY_SEPARATOR instead.
(Even the IIS version of WP 3.3 still uses a hard-coded '/' character.)

Sébastien Méric

the DIRECTORY_SEPARATOR trick works but as i do not want to modify the WP core files, maybe a better replacement for the CMB_META_BOX_URL could be :
define( 'CMB_META_BOX_URL', trailingslashit( str_replace( str_replace( '/', '\\', WP_CONTENT_DIR ), WP_CONTENT_URL, dirname(__FILE__) ) ) );

Sébastien Méric

well, it seems that even this "solution" is not working properly, so, until you have a better idea, i'll keep on using :
define( 'CMB_META_BOX_URL', trailingslashit( str_replace( '\\', '/', str_replace( str_replace( '/', '\\', WP_CONTENT_DIR ), WP_CONTENT_URL, dirname(__FILE__) ) ) ) );

Bill Erickson
Collaborator

Instead of replacing all the / with \, can't you replace it with DIRECTORY_SEPARATOR?

I haven't tried your code, but wouldn't that make it ONLY work on PC's where the backslashes are used instead of slashes? By using DIRECTORY_SEPARATOR it uses the platform-specific separator.

Sébastien Méric

ok, i tried this and it is working :
define( 'CMB_META_BOX_URL', trailingslashit( str_replace( DIRECTORY_SEPARATOR, '/', str_replace( str_replace( '/', DIRECTORY_SEPARATOR, WP_CONTENT_DIR ), WP_CONTENT_URL, dirname(__FILE__) ) ) ) );
so 1st str_replace replaces the ABSPATH slash with DIRECTORY_SEPARATOR in WP_CONTENT_DIR. as it is a server path, it should have been used in the 1st place anyway...
2nd str_replace replaces the WP_CONTENT_DIR with WP_CONTENT_URL in the dirname(__FILE__) but then, on widows, we have a mix between slashes and back slashes so....
3rd str_replace replaces back slashes by slashes as we are dealing with a uri where there should be no back slashes.

does it seems correct to you ?
i may even try a iftest to see if i need this 2 extra str_replace when a strpos detects a back slash in WP_CONTENT_DIR ?

Gary Jones

I just noticed that my wp-config-sample.php file contains:

if ( !defined('ABSPATH') )
    define('ABSPATH', dirname(__FILE__) . '/');

Could you amend that in the wp-config.php of your local install to use DIRECTORY_SEPARATOR, to see if it fixes the problem (obviously, undo any tweaks you've made to the original CMB code too)?

Sébastien Méric

yes, i tried that the 1st time you mentioned the DIRECTORY_SEPARATOR constant existence but unfortunately ABSPATH is already defined at that time...

Jared Atchison
Collaborator

@GaryJones for the next release would you recommend we leave as is or incorporate some variation of the str_replace mentioned earlier?

Bill and I aren't much help on this issue since we both are on macs.

Gary Jones

I would scour core trac for any signs as to whether someone has brought up the fact ABSPATH should be using DIRECTORY_SEPARATOR and see what the outcome was. I can't believe this wouldn't have been the first time it would have come up.

Until a better confident solution exists, I would do something like:

define( 'CMB_META_BOX_URL', apply_filters( 'cmb_meta_box_url', trailingslashit( str_replace( WP_CONTENT_DIR, WP_CONTENT_URL, dirname( _FILE_ ) ) ) ) );

if PHP will allow that. That then allows those running on Windows to filter in the more complex version (which, if they publish it to a non-Windows system, shouldn't break anything, and they can just comment out the add_filter() line anyway) which you can provide in some documentation.

Alternatively, you might see if '\' === DIRECTORY_SEPARATOR and if so, give your definition as the more complex version, and if not, fallback to the current version.

David Higginbotham

@GaryJones I am using your method currently and am getting this back: ./style.css. I took it one step further, and playing with it I used this method:

define( 'CMB_META_BOX_URL', apply_filters( 'cmb_meta_box_url', trailingslashit(trailingslashit( str_replace( WP_CONTENT_DIR, WP_CONTENT_URL, dirname(__FILE__) ) ) ) ) );

this in return got me back:

D:\inetpub\acme_Staging\wp-content\themes\acme-two\lib\metabox/style.css

any ideas?

Jared Atchison
Collaborator

Are we confident enough to use one of these modified methods in the next version?

If not I'll just leave what's there. Alternately maybe we could have both in place and comment out the longer one, that way we can tell them to uncomment if they run into issues (and are using WAMP).

Thoughts?

Sébastien Méric

+1 for, at least, Gary's filter method, so we do no not have to modify the framework core source code, till a better solution is found (sorry but i can't figure out a better way than the str_replace trick...). and maybe a note in the readme file/wiki for windows users ? something like :

there is a known bug on WAMP. add this kind of filter to your functions.php for the framework to work :
add_filter( 'cmb_meta_box_url', 'windows_cmb_meta_box_url' );
function windows_cmb_meta_box_url( $url ){
return trailingslashit( str_replace( '\\', '/', str_replace( str_replace( '/', '\\', WP_CONTENT_DIR ), WP_CONTENT_URL, $url ) ) );
}

and i hope this notice will be seen...

Jared Atchison
Collaborator

The filter has been added to trunk. Whenever trunk merges with master we will note this filter in the documentation and close the thread :)

Gary Jones

It'll be worth noting that the add_filter() line needs to come before the call to require CMB, since once the constant is defined, the filter won't kick in any more.

Jared Atchison
Collaborator

@GaryJones - thanks for pointing that out, I'll make sure that gets into the wiki docs!

Alex Mills

I don't understand why this is so complicated. The fix is easy. :)

http://codex.wordpress.org/Function_Reference/plugins_url

define( 'CMB_META_BOX_URL', trailingslashit( plugins_url( '', __FILE__ ) ) );

Or even better, don't define and use a constant at all.

wp_enqueue_script( 'cmb-timepicker', plugins_url( 'js/jquery.timePicker.min.js', __FILE__ ) );

(You don't need to call wp_register_script() if you're just going to enqueue it.)

Gary Jones

The CMB library could be used in plugins or themes, and could be in any directory or subdirectory with that plugin or theme package. There's no function that can grab the URL from a random path, without replacing the one bit that is known (root) with the domain. By all means define the constant in your plugin however you wish, if you know it will be installed into a constant URL.

Jared Atchison
Collaborator

Yeah as Gary mentioned the problem is it's used in everything - core functionality plugins, themes, etc so it's hard to use strictly plugins_url()

Alex Mills

Oh, that certainly complicates things I guess. At the very least though you should be able to do this:

if ( false !== strpos( __FILE__, WP_PLUGIN_DIR ) ) {
    define( 'CMB_META_BOX_URL', trailingslashit( plugins_url( '', __FILE__ ) ) );
} else {
    // Current code
}

That would fix plugins but not themes.

Gary Jones

What if the Cmb was not at the top level of a plugin directory though? Does plugins_url() still manage to find it regardless?

Alex Mills

It's smart and it shouldn't matter where it is within the plugin directory.

I have it at /plugins/my-plugin/metabox/init.php right now and it works 100% fine.

Norcross
Collaborator

here's a possible setup that'll catch the standard and when it's in the plugin, then allow people to define their own locally without having to change it when they push to production

$local  = array('localhost', '127.0.0.1');
if(!in_array($_SERVER['HTTP_HOST'], $local)) {
    define( 'CMB_META_BOX_URL', apply_filters( 'cmb_meta_box_url', trailingslashit( str_replace( WP_CONTENT_DIR, WP_CONTENT_URL, dirname( __FILE__ ) ) ) ) );
} elseif ( false !== strpos( __FILE__, WP_PLUGIN_DIR ) ) {
    define( 'CMB_META_BOX_URL', trailingslashit( plugins_url( '', __FILE__ ) ) );
} else {
    define( 'CMB_META_BOX_URL', 'DEFINE YOUR CUSTOM HERE' );
}
Wycks

Hardcoded HTTP_HOST values are not a viable option, not everyone uses localhost or 127.0.0.1.

Windows doesn't care if you use ABSPATH so you should just let people define a path, you cannot guess where they are going to put it and on which OS.

//write your location down lazy pants
$im_lazy =  ABSPATH . 'wp-content/some-folder';
define( 'CMB_META_BOX_URL', $im_lazy );  

Or a worse alternative is to check for the OS;

if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') {
       //winblows
    define( 'CMB_META_BOX_URL', trailingslashit( str_replace( DIRECTORY_SEPARATOR, '/', str_replace( str_replace( '/', DIRECTORY_SEPARATOR, WP_CONTENT_DIR ), WP_CONTENT_URL, dirname(__FILE__) ) ) ) );
    
} else {
    define( 'CMB_META_BOX_URL', apply_filters( 'cmb_meta_box_url', trailingslashit( str_replace( WP_CONTENT_DIR, WP_CONTENT_URL, dirname( __FILE__ ) ) ) ) );
}
Norcross
Collaborator

@wycks that's why the 3rd option allows a user to define their own if it doesn't fit either of the other two base options.

sampc

@wycks thank you.

Justin Sternberg jtsternberg closed this November 29, 2013
Cor van Noorloos corvannoorloos referenced this issue in johnbillion/query-monitor March 05, 2014
Open

QM stuck on plugin activation #47

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.