im-richard / Scaffold

Extend, combine and compress CSS
http://github.com/IAMRichardT/Scaffold/wiki
161 stars 19 forks source link

AbsoluteUrls resolving paths problem #30

Closed mfernea closed 14 years ago

mfernea commented 14 years ago

At the moment relative paths without "../" like "images/image.jpg" or with multiple "../" like "../../images/image.jpg" are not resolved correctly.

Looking into the Scaffold_Extension_AbsoluteUrls class I've discovered few issues.

  1. up_directory should not be called in resolve_absolute_path or it should do nothing unless "../" exists in the path. As a quick solution "if (!$n) return $path;" should be the first line in up_directory.
  2. as $path has a trailing slash the last element of $explode is an empty string. So we want to remove $n + 1 elements from the end of $explode: $exploded = array_slice($exploded, 0, (count($exploded) - $n - 1));
  3. for consistency, up_directory should return a path maintaining the trailing slash. So after slicing the $exploded array it should add an empty element ($exploded[] = '';) so that implode will add that trailing slash.
  4. when resolve_absolute_path returns the path it should replace "../" with empty string not "/".

After making this modifications I've run tests for each case and it works.

Best regards!

anthonyshort commented 14 years ago

Awesome! Thanks for that. Do you have a fork and can make a pull request?

mfernea commented 14 years ago

I'm setting things up now.

anthonyshort commented 14 years ago

I've fixed this up in the latest push. All unit tests are passed. Let me know if you still have issues.