impresspages / ImpressPages

ImpressPages is php framework with admin panel. Build functional website in one hour.
http://www.impresspages.org
Other
502 stars 175 forks source link

Less Compiler: Builtin data-uri() method doesn't work correctly #708

Open arm1n opened 9 years ago

arm1n commented 9 years ago

Hi there!

Just wanted to use Less' builtin data-uri() method for background images, but could not get it to work unless I took a look at IP's LessCompiler. By changing relativeUrls option to true, Less parser correctly transforms the image into a data uri. In my opinion it shouldn't matter if relativeUrls are set to false or true, but after debugging Less.php's datauri() method itself I experienced that $this->currentFileInfo['entryPath'] contains an empty string, so this could probably be a bug within Less Parser. Do you have any ideas how to resolve this issue without changing relativeUrls option, as I think this would affect too many existing sites?

maskas commented 9 years ago

Not sure how to handle this. LESS compiler is a complex thing. Changing the default configuration is definitely a risky option.

jankus commented 9 years ago

LESS compiler is a 3rd party library. Unfortunately, it's not very flexible.

Moreover, to make all overrides work (the magic of includes we have now) it is set up as is. Changing relative path option will break that logic. It could be that library is buggy, that's why we had to define it in this way.

Sadly, but this is the only available PHP library for this task.

There are two options:

  1. check whether you can repeat this bug in standalone LessCompiler. If yes, talk with library author to come up with the best approach to fix it.
  2. Make a patch and create pull request to LessCompiler repository. If it is accepted by library owner, we'll be able to update it in our core, too.
arm1n commented 9 years ago

Thank you for your information! Just tested latest version of less.php both within IP and standalone. Doesn't seem to be an issue with less.php, I'll try to dig a little bit deeper into LessCompiler's setup, maybe there's a simple fix without breaking logic...

jankus commented 9 years ago

Check the compiler's version, too. Maybe in the core we use outdated version and problem you're facing is already fixed.

arm1n commented 9 years ago

I've replaced less.php in my current project with latest version, but unfortunately that doesn't make any difference leaving LessCompiler.php untouched.

arm1n commented 9 years ago

Ok after some further research I think it's a bug in less.php. I've added a ticket with an idea to fix that, so we have to wait if library owner will accept my changes or proposes a better solution. I'll update this ticket as soon as I got feedback.

maskas commented 9 years ago

+1