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

Nested @supports conditions don't render correctly #614

Closed
xzyfer opened this issue Nov 3, 2014 · 9 comments · Fixed by #626
Closed

Nested @supports conditions don't render correctly #614

xzyfer opened this issue Nov 3, 2014 · 9 comments · Fixed by #626

Comments

@xzyfer
Copy link
Contributor

xzyfer commented Nov 3, 2014

Nested @supports conditions do not render correctly.

Input

@supports ((foo: bar) or (foo: bar) or (foo: bar)) and (foo: bar) {
  div {
    bar: baz;
  }
}

Expected output

@supports (foo: bar) or (foo: bar) or (foo: bar) and (foo: bar) {
  div {
    bar: baz; } }

Actual output

@supports (: or ) and (foo: bar) {
  div {
    bar: baz; } }

Specs added sass/sass-spec#122.

@HamptonMakes
Copy link
Member

Actually, the test was written wrong for this one, @xzyfer. You have to use 3.4 to generate this correctly. Your test showed

@supports ((foo: bar) or (foo: bar) or (foo: bar)) and (foo: bar) {
  div {
    bar: baz;
  }
}

When it should be...

@supports (foo: bar) or (foo: bar) or (foo: bar) and (foo: bar) {
  div {
    bar: baz;
  }
}

I'm updating this test in the threefour branch at sass-spec

@HamptonMakes HamptonMakes reopened this Nov 6, 2014
HamptonMakes pushed a commit to sass/sass-spec that referenced this issue Nov 6, 2014
@HamptonMakes
Copy link
Member

Actually, let me move this over to master, so that you can test the single issue without dealing with all teh 3.4 issues (though, this is related to one major issue i've run into, methinks)

HamptonMakes pushed a commit to sass/sass-spec that referenced this issue Nov 6, 2014
@xzyfer
Copy link
Contributor Author

xzyfer commented Nov 6, 2014

Sorry I'm not following @supports (foo: bar) or (foo: bar) or (foo: bar) and (foo: bar) and @supports ((foo: bar) or (foo: bar) or (foo: bar)) and (foo: bar) have two very different meanings.

You're example worked in my original implementation (#612) but my case failed as described in the issue description.

According to the grammar supplied by the w3c my example is correct http://www.w3.org/TR/css3-conditional/#at-supports

It feels like this may be bug in Ruby sass. It also appears that Ruby sass does not support negation http://sassmeister.com/gist/7503101e47d9b4e1a65b

@xzyfer
Copy link
Contributor Author

xzyfer commented Nov 6, 2014

I've opened an issue on Ruby sass to track this sass/sass#1512

@mgreter
Copy link
Contributor

mgreter commented Jan 6, 2015

What's the status on this?

@mgreter
Copy link
Contributor

mgreter commented Mar 7, 2015

@xzyfer Added a comment to sass/sass#1512, IMHO you're correct on this one!

@mgreter mgreter added this to the 3.2 milestone Mar 11, 2015
@mgreter
Copy link
Contributor

mgreter commented Mar 11, 2015

I'm convinced now that this is a ruby sass bug. @xzyfer if you agree we can IMO close this (maybe see the example I posted in the issue you've opened). I already added it to the 3.2 milestone 😉

@xzyfer
Copy link
Contributor Author

xzyfer commented Mar 11, 2015

Yeah I'm certain it's a bug in Sass sass/sass-spec@c58ab22. :Ship:

@xzyfer xzyfer closed this as completed Mar 11, 2015
xzyfer added a commit to xzyfer/sass-spec that referenced this issue Mar 11, 2015
This appears to be the correct behaviour. Ruby Sass appears to have a bug.
sass/libsass#614 (comment)

This reverts commit c58ab22.
@mgreter
Copy link
Contributor

mgreter commented Mar 11, 2015

There is an interesting difference in @support and @media when it comes to this. Added another comment to the ruby sass issue to document it. Not sure what to make of it though. Maybe we could alert the user of possible unwanted errors, but lets see what ruby sass has to say.

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

Successfully merging a pull request may close this issue.

3 participants