Skip to content
This repository has been archived by the owner on Jan 9, 2018. It is now read-only.

Ignore fragment url()s #18

Closed
wants to merge 4 commits into from
Closed

Ignore fragment url()s #18

wants to merge 4 commits into from

Conversation

cyberdelia
Copy link

A library I'm using has the following URI value:

behavior: url(#default#VML);

This breaks collectstatic, as it looks for a file of that name. I imagine it would be harmless to ignore any url that is entirely a fragment (ie any URL starting with a #), as it wouldn't involve any remote requests.

Although #17 looks to offer a way to ignore such errors, I think this case should be ignored for everyone as it's obvious that no file can be ever be served with such a name.

@jezdez
Copy link
Owner

jezdez commented Nov 9, 2011

Sounds like a plan.

@jezdez jezdez mentioned this pull request Dec 15, 2011
@cyberdelia
Copy link

Also this urls are failing :

url(web-font.eot?#iefix)

And SVG id in url :

url(web-font.svg#webfontIyfZbseF)

And also try to rewrite data-uri :

url(data:font/woff;charset=utf-8;base64,d09GRgABAAAAADJoAA0AAAAAR2QAAQAAAAAAAAAAAAA)

And it's a common pratice used for cross-platform web-fonts support.

It would be nice to not ignore them (except for data-uri), since they are font or others files behind them.

@jezdez
Copy link
Owner

jezdez commented Dec 15, 2011

Okay, sounds like a plan then:

  • ignore paths that are only fragments or start with data:
  • re-attach the fragments for files that have them

@willhardy
Copy link
Author

I tried to write some tests for this, but the presence of the #ed entries broke every other test. Does it need its own test environment or is it ok like this (because when this issue is fixed it won't break the other tests)?

# Handle directory paths
return name
# Handle directory fragments
return fragment
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might no be a good thing, but I see no use case for url() and a directory.

@cyberdelia
Copy link

I've been giving a try to this issue myself, it's not perfect but more acceptable than raising an error :|

hub has made this issue an pull request, I hope you don't mind, I will try to reverse that if it's the case.

@jezdez jezdez closed this in a823781 Dec 26, 2011
@cyberdelia
Copy link

Great! Thanks.

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

Successfully merging this pull request may close these issues.

3 participants