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

Minify Debug Mode: CSS line numbers debug data problem when source contatins comment with url #37

Closed
wants to merge 1 commit into from

Conversation

@SultanICQ
Copy link

SultanICQ commented Sep 10, 2019

If you have a block comment in one line with an url inside it, the code incorrectly thinks the comment is not finished.

This is due to the double slash in http:// that gets trim if at the same line there is an */ following it.

For example, this line:

/* Generated by Font Squirrel (http://www.fontsquirrel.com) on May 30, 2013 */

Gets converted to:

/* Generated by Font Squirrel (http:

So it thinks the comment is not finished.

That behaviour breaks web rendering because large portions of valid CSS get deactivated inside a comment when combining them inside the big css with line comments.

@maxicus maxicus changed the title Solved css combination failures when using urls inside single line comments Minify Debug Mode: CSS line numbers debug data problem when source contatins comment with url Sep 11, 2019
@bwmarkle

This comment has been minimized.

Copy link
Contributor

bwmarkle commented Sep 11, 2019

Hi @SultanICQ,

Thanks for the PR! I just wanted to touch base and let you know we'll be taking a look at your pull request shortly, and should have an update for you soon.

Thanks,

  • Brad
@bwmarkle

This comment has been minimized.

Copy link
Contributor

bwmarkle commented Sep 12, 2019

Hi @SultanICQ,

After talking with our lead developer on this one, he mentioned to me that the file where the bug is contained, lib/Minify/Minify/Lines.php, is actually apart of the mrclay/minify library. It looks like your fix may work, but generally we prefer to use the library's updated code.

Are you able to replace your lib/Minify/Minify/Lines.php file with what's here https://github.com/mrclay/minify/blob/master/lib/Minify/Lines.php and help us test if that resolves the issue?

Thanks,

  • Brad
@SultanICQ

This comment has been minimized.

Copy link
Author

SultanICQ commented Sep 17, 2019

Hello! It works perfectly.

Looking at the code I see that already take into account this case within others.

Regards,

@bwmarkle

This comment has been minimized.

Copy link
Contributor

bwmarkle commented Sep 20, 2019

Much appreciated @SultanICQ for taking the time to help us test! We're going to wrap up our own testing on this, and hopefully we'll be able to get this fix released in our next patch release.

@maxicus

This comment has been minimized.

Copy link
Contributor

maxicus commented Sep 23, 2019

Thank you for you help!
Applied that by c037924

@maxicus maxicus closed this Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.