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

CSSRewriter should support rewriting @import #446

Open
humphd opened this issue Aug 26, 2015 · 10 comments
Open

CSSRewriter should support rewriting @import #446

humphd opened this issue Aug 26, 2015 · 10 comments
Labels

Comments

@humphd
Copy link

humphd commented Aug 26, 2015

We support some, but not all the possible ways of doing URL imports in CSS (see https://developer.mozilla.org/en/docs/Web/CSS/@import):

@import url;                      or
@import url list-of-media-queries;

/* examples */
@import url("fineprint.css") print;
@import url("bluish.css") projection, tv;
@import 'custom.css';
@import url("chrome://communicator/skin/");
@import "common.css" screen, projection;
@import url('landscape.css') screen and (orientation:landscape);

The ones that don't use url() aren't going to work with our parsing, see https://github.com/humphd/brackets/blob/bramble/src/filesystem/impls/filer/lib/CSSRewriter.js#L19-L72.

@Pomax, if you get bored, this would be something you could help with.

@Pomax
Copy link

Pomax commented Aug 26, 2015

@gideonthomas gideonthomas added the P3 label Jan 8, 2016
@humphd
Copy link
Author

humphd commented Feb 17, 2017

Someone should confirm whether I'm right or wrong on these, and let's close or fix. @Pomax I can't remix your thing above, so I'm not sure what it is.

@Pomax
Copy link

Pomax commented Feb 17, 2017

yeah this project no longer seems to exist. Older Thimble maybe?

@aayushsanghavi
Copy link

@humphd I tried importing a css file using import url("file name") <name of the element with media query> and it didn't work but when I tried import url("<name_here>") the media query worked.
So I take it that, this is still an issue. Can I work on this?

@humphd
Copy link
Author

humphd commented Apr 22, 2017

@aayushsanghavi sure, if you want to give it a try. The code in question is here: https://github.com/mozilla/brackets/blob/master/src/filesystem/impls/filer/lib/CSSRewriter.js#L20-L89. That should find and rewrite all relative file paths in a url(...) to use a Blob URL.

Further, we need to deal with the special case of there being no url( prefix at all, and just the filename in quotes.

@aayushsanghavi
Copy link

aayushsanghavi commented Apr 22, 2017

Yes I tried that too, the @import "file name" and it didn't work.
I'll look into the code soon, Wednesday mostly. Thanks for assigning it.

@aayushsanghavi
Copy link

@humphd I started work on this issue and I've added functionality of using @import "cssfile.css". For the others could you suggest to me how I should proceed? I'd like to have a clear picture of the steps I would have to follow in order to implement it.
The way I see it, this will involve use of regex for extracting the names of media-queries and then replacing the content of the media-queries with its actual css content from that file.

@aayushsanghavi
Copy link

Sorry, that this issue has been kept waiting for quite some time. I went out of town for a couple of weeks. I just got back 2 days ago so I'll be working on this again. @humphd regarding my previous comment, what do you think is the best way to go ahead with the css imports? What I had suggested earlier looks quite complicated.

@humphd
Copy link
Author

humphd commented Jun 8, 2017

You're going to need to do one of two things. Either:

  1. Alter the regex var urlRegex = new RegExp('url\\([\\\'\\"]?([^\\\'\\"\\)]+)[\\\'\\"]?\\)', 'g'); to include an optional @import prefix (might be hard or impossible)
  2. Write a new regex specifically for the @import <string | url> case, and duplicate this section using this second regex. This would find and accumulate all URLs that are not caught by the first, and you might choose to focus solely on the @import <string> case, since the @import url(...) case will be caught by 1. If it were me, I'd probably do this option.

You don't need to do anything other than that, since the existing code will handle everything else: you just need to find all these URLs.

@Pomax is the best regex'er I know, so you might get his advice here.

@aayushsanghavi
Copy link

Thanks @humphd. I'll get onto it then. I'll let you know if I have doubts or if I'm stuck somewhere.

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

No branches or pull requests

4 participants