-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Bug fix when using svg-gradient in a mixin in a nested folder #2361
Conversation
Tests which expose the bug in less#2360
I don't entirely understand how the tests are setup, so there are some failing tests still but they appear to be isolated to base64 encoding options.
I figured out that the test would still fail if I call the import in the urls.less file. That way I don't need to figure out all the setup required to test this bug. - less#2360
Looks like appvoyer failed to run |
I think its a random appveyor failure. Ive restarted. |
@@ -79,6 +79,13 @@ contexts.Eval.prototype.isPathRelative = function (path) { | |||
return !/^(?:[a-z-]+:|\/)/i.test(path); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already setup to return false (path not relative) for data-uris. Any idea why it isn't working?
I don't want to add a new isPathExternal function when essentially !isPathRelative is already setup to do that job..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't dig into that regex earlier since the method name had me believe it was looking for things like ../../folder
, ~/folder
, or /folder
. I didn't realize that it was doing this by checking that it was not something else.
So, I do see how it would fail for data:image
, however 'data:image....
is getting passed in. Note the leading quotation mark. Would it make more sense to strip out that quotation mark in this method or require the caller to strip it out? Or is this an issue with the Quoted
prototype?
I pulled your branch, reverted your fix and then fixed it by changing the svg gradient function to return a Quoted rather than an Anonymous - that way the function is called without the starting quote - matching the way the url node is used everywhere. |
Awesome thanks, when will it be released? |
This weekend latest is my plan. |
Sounds good thanks again. |
Fixes #2360
There are a few test failures right now, but I'm positive that it is due to test setup regarding a base64 encoding option. I'm not sure how to setup the test properly, but at least it is passing for the original case I was going for. Any advice to setup the test properly would be appreciated.