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

Relative image URIs are getting broken in symlinking scenarios #17

Open
lkraav opened this issue Aug 19, 2013 · 10 comments
Open

Relative image URIs are getting broken in symlinking scenarios #17

lkraav opened this issue Aug 19, 2013 · 10 comments
Labels

Comments

@lkraav
Copy link

lkraav commented Aug 19, 2013

"images/bigback_inner.jpg" becomes "nt/themes/my-theme/images/bigback_inner.jpg".

it's pretty clear "^nt" is a cut off version of "wp-content".

probably something to do with

795                     $contents = Minify_CSS_UriRewriter::rewrite( $contents, ABSPATH . $src_dir_path );
@lkraav
Copy link
Author

lkraav commented Aug 21, 2013

Could anyone tell me what a working image URL is suppose to look like? Is it still served behind /_minify/ or not?

@lkraav
Copy link
Author

lkraav commented Aug 21, 2013

I think I have a way to reproduce the URL rewrite issue: symlink your active theme's directory from somewhere else. This should confuse the rewriter.

@lkraav
Copy link
Author

lkraav commented Aug 22, 2013

And last but not least, my symlink basename is different from realpath's basename. So there :)

@lkraav lkraav closed this as completed Aug 22, 2013
@lkraav lkraav reopened this Aug 22, 2013
@westonruter
Copy link
Contributor

A working image URL shouldn't be served under /_minify/, but rather from from a theme or plugin subdirectory.

The code in question is this:

// Rewrite relative paths in CSS
$src_dir_path = dirname( parse_url( $src, PHP_URL_PATH ) );
if ( 'styles' === $type && is_dir( ABSPATH . $src_dir_path ) ) {
    $contents = Minify_CSS_UriRewriter::rewrite( $contents, ABSPATH . $src_dir_path );
}

However, if the $src originally registered is a valid URL to to a dependency, then I don't see how this would cause problems with symlinked themes. The problems I've encountered with symlinks were caused specifically by plugin_dir_url().

If you turn off dependency minification, what is the normal stylesheet src URL that gets enqueued? Where is the symlink pointing?

@lkraav
Copy link
Author

lkraav commented Aug 22, 2013

I'm not sure why you're asking about the stylsheet src url. It is built normally with get_stylesheet_directory_uri() . "/style.css". DM is able to correctly minify and enqueue the stylesheet and it loads correctly.

Problem is with relative image css rules inside the stylesheet:

background: url(images/bigback_inner.jpg);

becomes

background: url(nt/themes/mytheme/images/bigback_inner.jpg);

The symlink structure is:

$ pwd
~/public_html/customer/wp-content/themes
$ ls -l mytheme
lrwxrwxrwx 1 markitek markitek 60 Nov 29  2012 mytheme -> /home/markitek/www/wp-content/themes/mytheme-branch

It's a bit of an edge case indeed. I don't usually do this, it's this long running project coming from a legacy situation.

@westonruter
Copy link
Contributor

The stylesheet URL is used as the base URL for relative URLs inside of the stylesheet, per the snippet shown above. Perhaps it's a problem inside of the Minify_CSS_UriRewriter::rewrite() method itself, probably something to do with realpath resolving the symlinks when they should be left symbolic?

@lkraav
Copy link
Author

lkraav commented Aug 23, 2013

You are correct. $debugText has:

docRoot    : /home/markitek/public_html/loghouse
currentDir : /home/markitek/public_html/anna-maria/loghouse-theme-anna-maria

This is symlinking the user's homedir branch into themes catalog. If currentDir is used in determining the rewrite, it should definitely be left as a symlink.

@lkraav
Copy link
Author

lkraav commented Aug 23, 2013

Maybe this https://gist.github.com/Tarendai/2971872 is of help for brainstorming and https://core.trac.wordpress.org/ticket/16953

@lkraav
Copy link
Author

lkraav commented Aug 23, 2013

There's another issue: plugins dir, that I also have symlinked for a few WP instances, causes docroot stripping to give a bad result.

docRoot    : /home/markitek/public_html/loghouse
currentDir : /home/markitek/public_html/wp-content/plugins/wordpress-seo/css

@lkraav
Copy link
Author

lkraav commented Aug 23, 2013

Everything works with this simple patch:

- 49         self::$_currentDir = self::_realpath($currentDir);
+ 49         self::$_currentDir = str_replace( '//', '/', $currentDir );

_realpath() looks very complicated hoopla to me and I'm not familiar with the reasoning behind it, so I'm doing this as a workaround for now. '//' needs to be handled because adding ABS_PATH . $src_dir_path creates '^//' and if you leave it in, it will be interpreted as a protocol-agnostic URI.

westonruter added a commit that referenced this issue Dec 12, 2013
cc5a003 Reference master branch in TravisCI build badge
2e82c85 Merge commit '85f0b269038cb772d9353f7236a28757c10295af'
234a146 Merge pull request #25 from GaryJones/patch-1
5c6fae7 Remove smarttabs option from .jshintrc
9fcac99 Add Generic.ControlStructures.InlineControlStructure
218f007 Merge commit '6a9b2c7f6a8044f337152fb406ebe58f1c26b5ac' into develop
894d120 Merge pull request #18 from x-team/issue-16
de1db88 Remove wordpress- from url
5581e97 Get latest trunk file from github
256b0a0 Embed YouTube demo video in readme; add markdown support
9d1d025 Add base phpcs ruleset for plugins
5de1b8e Update .jshintrc to match new in core
ab7b9cd Update .tavis.yml to check against master and latest. This closes #16 and #15
9aa3e33 Merge pull request #17 from jonathanbardo/master
63a11a4 Correct small typo
8e3bab1 Merge pull request #15 from jonathanbardo/master
ed0b379 Update WP version to latest stable release
6956ba1 Merge pull request #13 from jonathanbardo/master
a864385 Update WP_VERSION to 3.7
0c545c3 Merge commit '4cd538fd0b8e6fcbe68cd4e4a8e39206103023b2' into develop
4cd538f Merge branch 'master' of github.com:x-team/wp-plugin-dev-lib
258cdf9 Fix reference from jshint to phpcs
c9f45eb Add formatting
7d26511 Add git-subtree instructions
528bd0e Only apply jshint to staged JS files
9c1e6ea Update svn-push to work with svn repo out of git repo
86d46a9 Explicitly reference bash (not dash) for pre-commit hook
64a6d09 Move WordPress.org SVN checkout outside of Git root
a4aa443 Fix symlinked dirs for Travis; remove PHP 5.2 for Travis
fd97d04 Finish with 5.2 compat; exclude bin from syntax check
ff49315 Improve relative paths for phpunit; update install-wp-tests.sh
c213c44 Run Travis tests on PHP 5.2
4eaa28e Account for renamed files in pre-commit hook
8001c36 Eliminate PHP 5.3 requirement, replacing namespaces and closures
774bdc3 Check modified files, pass select JS to jshint, run phpunit in vagrant
bc955c7 Fix pre-commit detection of staged_php_files
d0194bb Add PHP syntax checks for travis-ci and pre-commit hook
c98d9c4 Merge branch 'master' of github.com:x-team/wp-plugin-dev-lib
2e1b928 Remove dependency on WP_CLI
0e07a75 Note how add .travis.yml and .jshintrc to host plugin via symlinks
da3555c Also check for presence of phpunit.xml.dist
0ae29af Update pre-commit hook so PHPCS only looks at staged PHP files
00d36fb Merge commit '7ef48bd5aa6680d02589a9f5697b35cd9a9520ca' into develop
f5f2c1c Update pre-commit hook to look for staged php files
3ddac64 Revamp pre-commit hook, adding jshint
9f7df05 Rename .jshint => .jshintrc
817f4c2 Add travis-ci with support for jshint, phpcs, phpunit
a3a984d Add jshint and fix warnings
730cf4f Merge commit '32585ea10effb3de33aa7136424364359c199969'
1095520 Apply phpcs fixes

git-subtree-dir: bin
git-subtree-split: cc5a003aca81eb3af6c01f556446168e3e71263d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants