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

Emulate Ruby Sass' url() parsing semantics #1595

Merged
merged 2 commits into from
Oct 14, 2015

Conversation

xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Oct 12, 2015

We've had countless bugs and regressions with parsing url(). This
patch is complete refactor of our url() parsing semantics to closer
match that of Ruby Sass.

TODO

  • implement the correct handling of interpolants in url()
  • implement the correct semantics for url-prefix()
  • determine / implement the correct semantics for @import url()
  • replace uri functions / variables with their real_ prefixed versions

Update: I've reduced the scope of this PR to the regression at hand. This update to url() semantics will need to applied to url-prefix() (#1596) and @import url() (#1597) for 3.4.

Fixes #674
Spec sass/sass-spec#539

@xzyfer
Copy link
Contributor Author

xzyfer commented Oct 12, 2015

The spec currently failing is with regards to insignificant whitespace. Not entirely sure how to handle this yet.

a {
  b: url(data:image/png;base64,iVI= );
}

Ruby Sass

a {
  b: url(data:image/png;base64,iVI= );
}

LibSass

a {
  b: url(data:image/png;base64,iVI=); // no whitespace
}

@xzyfer xzyfer force-pushed the feat/url-for-reals branch 2 times, most recently from a363780 to 937e459 Compare October 13, 2015 11:52
We've had countless bugs and regressions with parsing url(). This
patch is complete refactor of our url() parsing semantics to 100%
match that of Ruby Sass.

Fixes sass#674
Spec sass/sass-spec#539
@xzyfer xzyfer force-pushed the feat/url-for-reals branch from a42df42 to 4d3fe32 Compare October 13, 2015 13:24
xzyfer added a commit that referenced this pull request Oct 14, 2015
Emulate Ruby Sass' url() parsing semantics
@xzyfer xzyfer merged commit 4f62bea into sass:master Oct 14, 2015
@xzyfer xzyfer deleted the feat/url-for-reals branch October 14, 2015 07:03
@mgreter
Copy link
Contributor

mgreter commented Jan 17, 2016

Just retested and latest ruby sass seems to give:

a {
  b: url(data:image/png;base64,iVI=);
}

So this might be fixed already!?

@xzyfer
Copy link
Contributor Author

xzyfer commented Jan 17, 2016

This was fixed in 3.3.

replace uri functions / variables with their real_ prefixed versions

Is fixed in 3.3.3.

@xzyfer
Copy link
Contributor Author

xzyfer commented Jan 17, 2016

determine / implement the correct semantics for @import url()

Is fixed in 3.3.3.

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

Successfully merging this pull request may close these issues.

Users cannot safely redefine url()
2 participants