thom4parisot / wp-less

WordPress plugin which seemlessly compiles, caches and rebuilds your LESS stylesheets.
https://wordpress.org/plugins/wp-less/
88 stars 40 forks source link

Doesn't work with multisite #52

Open royduin opened 10 years ago

royduin commented 10 years ago

Hi,

I'm getting errors when I'm using WP-LESS with WP multisite. I've created a fix for it:

Replace line 87 in "Stylesheet.class.php" from: $this->source_path = WP_CONTENT_DIR.preg_replace('#^'.content_url().'#U', '', $this->stylesheet->src); To: $this->source_path = WP_CONTENT_DIR.str_replace('/wp-content','',parse_url($this->stylesheet->src,PHP_URL_PATH));

It would be nice if you change this in a feature version.

lkraav commented 10 years ago

Have you tried https://github.com/sanchothefat/wp-less instead? It's the better plugin right now.

thom4parisot commented 10 years ago

There is an element of answer in #36. Is the proposed solution somewhere in the comments helpful enough to solve the problem?

I thought it has been added in the plugin's code but apparently I missed it.

royduin commented 10 years ago

@lkraav: Haven't tried, but is it possible with that one to update through Wordpress as well?

@oncletom: Sorry, didn't see it. As written in my first post, I've already fixed it, but I suggest to put my fix in the next version.

thom4parisot commented 10 years ago

Sorry I wrote the answer meanwhile you were editing. I thought this was fixed.

In any case, hardcoding to /wp-content is not a solution at all. I'd rather fix the content_url() substitution rather than making this change the way back.

royduin commented 10 years ago

But will this be fixed in a feature version?

thom4parisot commented 10 years ago

content_url() outputs the following.

If you could provide those 3 values, we could figure out how to address the problem nicely:

Thanks :+1:

royduin commented 10 years ago

The problem is that $this->stylesheet->src returns a full url so the preg_replace doesn't work.

thom4parisot commented 10 years ago

@royduin yeah I know but that does not help me to fix the problem :-)

RalphGL commented 10 years ago

Today I gave LESS-WP a try - but it doesn´t compile any less-files on my multisite installation, so what can we do to support for a fast solution of the problem?

thom4parisot commented 10 years ago

As stated above, I need to understand what is multisite configuration generating.

So if you could provide those 3 values, we could figure out how to address the problem nicely:

gereby commented 10 years ago

did anyone develop a solution to this so far?

thom4parisot commented 10 years ago

What does content_url() return for a multisite? Because it is supposed to be designed to handle both single and multisites install of WordPress.

wtaeke commented 9 years ago

Thanks @royduin - your fix is a lifesaver!!

thom4parisot commented 9 years ago

@wtaeke is there any chance you can tell me what is the value of content_url() for a multisite blog? Because /wp-content is a hardcoded value which can be changed through wp-config.php — opting for the aforementioned proposed change would break wp-less for those people.

Thanks!

tyrann0us commented 9 years ago

Hi @oncletom, this issue is still present.

$this->stylesheet->src // http://example-b.com/wp-content/themes/theme-name/less/style.less content_url() // http://example-a.com/network-site-b/wp-content $this->source_path // C:\xampp\htdocs\example-a.com/wp-contenthttp://example.com/wp-content/themes/theme-name/less/style.less

Hope this helps.

I have my Network Site B mapped to example-b.com using WordPress MU Domain Mapping. Any chance to see a fix for this? Thanks!

tyrann0us commented 9 years ago

@oncletom Unfortunately this problem still exists in version 1.8.0. Could you please provide a bugfix? Thank you!

thom4parisot commented 9 years ago

please provide an bugfix

Please don't be rude, as this is not going to help.

tyrann0us commented 9 years ago

Sorry, I didn't want to be rude! Blame my bad english skills ;)

thom4parisot commented 9 years ago

In any case, for what I can see:

A more flexible system is required. Like replacing the path from the value of WP_CONTENT_DIR (minus ABSPATH) to avoid dealing with absolute paths.

To be honest I don't have PHP nor wordpress installed on my machine anymore – you'll have to help me figuring out a solution. Try out if the following works.

$wp_content_dir = str_replace(ABSPATH, '', WP_CONTENT_DIR);

$this->source_path = WP_CONTENT_DIR.str_replace($wp_content_dir,'',parse_url($this->stylesheet->src, PHP_URL_PATH));

Note: it is wrong to hardcode wp-content folder as some people will customise this folder by altering the WP_CONTENT_DIR constant.

TrevorMW commented 8 years ago

@oncletom I've got domain mapping on a multisite with external domains coming in, and I experienced the issue that this thread started with, and this code seems to have fixed the issue when replacing line 87 in Stylesheet.class.php with it. Probably not the most concrete fix, but it's a good patch for now

$this->source_path = WP_CONTENT_DIR.str_replace($wp_content_dir,'',parse_url($this->stylesheet->src, PHP_URL_PATH));
schnoggo commented 8 years ago

@oncletom Your proposed patch works fine. I'm running the "other" flavor of multisite (folder) and this solved the problem for me. Will you just roll it in, or do you want one of us to make a pull request?

thom4parisot commented 8 years ago

@schnoggo yes please, go ahead :-)

schnoggo commented 8 years ago

Set up some more sites on my test multisite and ran into further errors. @oncletom @TrevorMW : Any reason to not use get_stylesheet_directory() instead of basing off of WP_CONTENT_DIR?

thom4parisot commented 8 years ago

@schnoggo from the top of my head, this is what I tried but it had different behaviours between MU websites and non-MU ones. Or they did not provide exactly the same value.

I'm up for any solutions which works for MU and non-MU websites, with standard and custom WP_CONTENT_DIR values.

schnoggo commented 8 years ago

Here's a (sanitized) dump of what's going on:

$this->stylesheet->src: http:somesite.com/{sub-site}/wp-content/themes/{mainsite}/style.less $wp_content_dir: wp-content get_stylesheet_directory_uri(): http:somesite.com/{sub-site}/wp-content/themes/{mainsite} get_stylesheet_directory(): /dom13937/wp-content/themes/{mainsite} WP_CONTENT_DIR: /dom13937/wp-content ABSPATH: /dom13937/

So the catch is that enque is using a URI, but you need a local path to set/check the mod time of the file. How about if we calculate the wp-content path like you're doing, then preg_replace to get the part AFTER that (the path within the theme folder) and then tack those two together to get the local path?

Can anyone think of a case where that would cause a problem? Pretty sure any site in a multi-site will have the same wp-content path.

schnoggo commented 8 years ago

All of these solutions assume the path is in the theme folder... (Or at least the wp-content directory...) No one's tried it with external .less files? :)

schnoggo commented 8 years ago

Here's my proposed patch:

    // path to local content dir does not match URI in multisite.
    $wp_content_dir = str_replace(ABSPATH, '', WP_CONTENT_DIR); // get the 'wp-content' part of the path
    $lessfile_in_theme = preg_replace ('#^.*?' . DIRECTORY_SEPARATOR . $wp_content_dir . DIRECTORY_SEPARATOR . '(.*)$#', '$1', $this->stylesheet->src, 1); // the part after 'wp-content'
    $this->source_path = WP_CONTENT_DIR  . DIRECTORY_SEPARATOR . $lessfile_in_theme;
 //   $this->source_path =    WP_CONTENT_DIR.preg_replace('#^'.content_url().'#U', '', $this->stylesheet->src);

I tested this in a regular WordPress install, multisite (path-based) root site and multisite child site. I don't have an easy way to domain-based multisite.

@TrevorMW any chance you could try this patch your domain-based system?

schnoggo commented 8 years ago

@tyrann0us Could you see if this solves your issue? (If it does, I'll post a pull request right away.) Replace configurePath() around line 83 in /lib/Stylesheet.class

 protected function configurePath()
  {
    $target_file = $this->computeTargetPath();
    // path to local content dir does not match URI in multisite.
    $wp_content_dir = str_replace(ABSPATH, '', WP_CONTENT_DIR); // get the 'wp-content' part of the path
    $lessfile_in_theme = preg_replace ('#^.*?' . DIRECTORY_SEPARATOR . $wp_content_dir . DIRECTORY_SEPARATOR . '(.*)$#', '$1', $this->stylesheet->src, 1); // the part after 'wp-content'
    $this->source_path = WP_CONTENT_DIR  . DIRECTORY_SEPARATOR . $lessfile_in_theme;
    $this->source_uri =     $this->stylesheet->src;
    $this->target_path =    self::$upload_dir.$target_file;
    $this->target_uri =     self::$upload_uri.$target_file;

    $this->setSourceTimestamp(filemtime($this->source_path));
  }
sliu2016 commented 8 years ago

I have same Problem with Multisite. I used Plugin "wordpress-mu-domain-mapping", so i can www.multisite.com/site1 -> www.site1.com mapping.

content_url() return http://www.multi.com/site1/wp-content, but $this->stylesheet->src is http://www.site1.com/wp-content.....

My solution ist use filter content_url

add_filter('content_url', 'content_url_for_less', 10, 2);
function content_url_for_less($url, $path = ''){
    $siteurl = site_url();
    if (stripos($url, $siteurl) === false){ //multi site with domain mapping
        $url = $siteurl . '/wp-content';
        if ( $path && is_string( $path ) )
            $url .= '/' . ltrim($path, '/');
    }

    return $url;
}
w-morgan commented 8 years ago

@sliu2016 Your solution totally worked for me!! I was freaking out, I owe you a beer :) Thanks a lot!

tyrann0us commented 7 years ago

@schnoggo after over a year I finally came back to this issue ... Your solution works for me, please make a PR! Thanks.