fix: Correctly enclose negated downleveled interval media queries in parens

[LeoniePhiline]

Fixes #320

Changes

Tests

Targets and range syntax

I changed relevant tests in lib.rs from firefox 60 and 85 to firefox 62 and 63, as media feature range syntax was introduced with firefox 63.
This takes into account that lightningcss currently treats range syntax and interval syntax as two different features. See also #331

It might be that some browsers first implemented range syntax without interval syntax, and added interval syntax later. However, this is not documented in the compatibility tables. Lightningcss considers interval syntax as not supported anywhere, even though, in reality, current Gecko and Blink (but not WebKit, apparently) do support interval syntax today.

As interval syntax is now (if this PR be merged) implemented by rendering a MediaCondition::Operation of two MediaFeature::Range features, the output depends on the range syntax target support. Therefore, there are now two versions of each interval media feature query test - one using range syntax (firefox >= 63) and one using traditional syntax (firefox <= 62).

Test placement

I had been unsure where to put the new, initially failing test reproducing #320. Placing it right in the media_query module seemed to be the right thing, but there is otherwise only a test for and(). All other tests - including media query tests - appear to be located in lib.rs. Should my added tests from media_query.rs be migrated over into lib.rs?

Complex implementation, benefits and possible alternatives

The implementation of the fix might appear somewhat contrived at first sight. Yet in terms of behavior it seems to me to be the cleanest solution.

The issue is that if the downleveling only happens in impl<'i> ToCss for MediaFeature<'i> (as before the PR), then we cannot know if the feature is negated, the trait contract does not offer passing further options, except for the printer dest.

Alternatives

A) Add a field to Printer

The Printer struct does have an in_calc boolean already - would it be the right thing to add an in_negated_media_condition field? This looks a little like abuse to me. However, is the implementation in this PR the better choice?

Adding in_negated_media_condition to struct Printer would be an option. Feels wrong, but less complex.

B) Always wrap expanded interval in parens in impl<'i> ToCss for MediaFeature<'i>

Any expanded media feature interval could be enclosed in parens.
However: This outputs unnecessary parens for non-negated queries.

E.g. @media (200px >= width >= 100px) would result in @media ((max-width: 200px) and (min-width: 100px)), where @media (max-width: 200px) and (min-width: 100px) would be sufficient.

C) Using the solution provided in the PR you are currently looking at

Complex code, but clean output.

[devongovett]

Thanks for putting together all these options and sorry for my slow response. I decided to completely remove the InParens variant from the AST, as it was causing some issues. Instead, we detect when parentheses are needed during printing, which simplifies transforms because output is guaranteed to be correct. See #343.