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

Fixed relative @import URLs #463

Closed
wants to merge 2 commits into from

Conversation

SGudbrandsson
Copy link
Contributor

I had a problem importing relative URL's so I had to do a bunch of debug.

Found the problem on line 267 - it needed "parsedUrl.pathname"

Here's stuff from the debug (so you can verify on your end)

root@testhost:# cleancss -d --root http://jeffwalker.com/wp-content/themes/salient-child/ http://jeffwalker.com/wp-content/themes/salient-child/style.css?ver=4.1.1
http://jeffwalker.com/wp-content/themes/salient-child/style.css?ver=4.1.1
Inlining remote stylesheet: http://jeffwalker.com/wp-content/themes/salient-child/style.css?ver=4.1.1
../salient/style.css
Inlining remote stylesheet: http://jeffwalker.com/salient/style.css
fonts/bebas-neue/stylesheet.css
Inlining remote stylesheet: http://jeffwalker.com/fonts/bebas-neue/stylesheet.css
Original: 28258 bytes
Minified: 23304 bytes
Efficiency: 17.53%
Time spent: 96ms
ERROR: Broken @import declaration of "http://jeffwalker.com/salient/style.css" - error 404
ERROR: Broken @import declaration of "http://jeffwalker.com/fonts/bebas-neue/stylesheet.css" - error 404
root@testhost:
# cleancss --version
3.0.10

inliner: { timeout: 5000, request: {} },
rebase: true,
relativeTo: 'http://jeffwalker.com',
root: '/root',
sourceTracker: { sources: [ 'http://jeffwalker.com/wp-content/themes/salient-child/style.css' ] },
warnings: [],
visited:
[ 'http://jeffwalker.com/wp-content/themes/salient-child/style.css',
'http://jeffwalker.com/salient/style.css' ],
afterContent: false,
shallow: false,
isRemote: true }

*** AFTER FIX ***

root@testhost:~# cleancss -d http://jeffwalker.com/wp-content/themes/salient-child/style.css -o jeff.css
Inlining remote stylesheet: http://jeffwalker.com/wp-content/themes/salient-child/style.css
Inlining remote stylesheet: http://jeffwalker.com/wp-content/themes/salient/style.css
Inlining remote stylesheet: http://jeffwalker.com/wp-content/themes/salient-child/fonts/bebas-neue/stylesheet.css
Original: 248065 bytes
Minified: 202793 bytes
Efficiency: 18.25%
Time spent: 353ms

I had a problem importing relative URL's so I had to do a bunch of debug.

Found the problem on line 267 - it needed "parsedUrl.pathname"

Here's stuff from the debug (so you can verify on your end)

root@testhost:~# cleancss -d --root http://jeffwalker.com/wp-content/themes/salient-child/ http://jeffwalker.com/wp-content/themes/salient-child/style.css?ver=4.1.1
http://jeffwalker.com/wp-content/themes/salient-child/style.css?ver=4.1.1
Inlining remote stylesheet: http://jeffwalker.com/wp-content/themes/salient-child/style.css?ver=4.1.1
../salient/style.css
Inlining remote stylesheet: http://jeffwalker.com/salient/style.css
fonts/bebas-neue/stylesheet.css
Inlining remote stylesheet: http://jeffwalker.com/fonts/bebas-neue/stylesheet.css
Original: 28258 bytes
Minified: 23304 bytes
Efficiency: 17.53%
Time spent: 96ms
ERROR: Broken @import declaration of "http://jeffwalker.com/salient/style.css" - error 404
ERROR: Broken @import declaration of "http://jeffwalker.com/fonts/bebas-neue/stylesheet.css" - error 404
root@testhost:~# cleancss --version
3.0.10



  inliner: { timeout: 5000, request: {} },
  rebase: true,
  relativeTo: 'http://jeffwalker.com',
  root: '/root',
  sourceTracker: { sources: [ 'http://jeffwalker.com/wp-content/themes/salient-child/style.css' ] },
  warnings: [],
  visited: 
   [ 'http://jeffwalker.com/wp-content/themes/salient-child/style.css',
     'http://jeffwalker.com/salient/style.css' ],
  afterContent: false,
  shallow: false,
  isRemote: true }




*** AFTER FIX ***

root@testhost:~# cleancss -d http://jeffwalker.com/wp-content/themes/salient-child/style.css -o jeff.css
Inlining remote stylesheet: http://jeffwalker.com/wp-content/themes/salient-child/style.css
Inlining remote stylesheet: http://jeffwalker.com/wp-content/themes/salient/style.css
Inlining remote stylesheet: http://jeffwalker.com/wp-content/themes/salient-child/fonts/bebas-neue/stylesheet.css
Original: 248065 bytes
Minified: 202793 bytes
Efficiency: 18.25%
Time spent: 353ms
@jakubpawlowicz
Copy link
Collaborator

@sigginet To be honest I've never used the --root switch that way :-)

Your change definitely makes sense. Could you add a test for it to https://github.com/jakubpawlowicz/clean-css/blob/master/test/protocol-imports-test.js ?

@SGudbrandsson
Copy link
Contributor Author

S
Test results:


root@testhost:/ble/clean-css-master# nodejs test/protocol-imports-test.js
··········································· ✓ OK » 43 honored (0.193s)
^Croot@testhost:
/ble/clean-css-master#


I tried using the --root switch to see if I could force the import to work properly .. it didn't (same behaviour with or without the switch) ... aaaanywho, now remote relative urls should be imported correctly.. :)

jakubpawlowicz added a commit that referenced this pull request Feb 17, 2015
@jakubpawlowicz
Copy link
Collaborator

@sigginet I've squashed your commits, cleaned up commit message, and merged it master manually.

Thanks a lot for the fix!

@SGudbrandsson
Copy link
Contributor Author

Kick-ass. Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants