Parse sizes attribute implementation

[srm09]

#10827 Original PR, somehow got merged with 0 changes. Reopened a new one, but please refer to the conversation from the original one.

---

This change is

[highfive]

Heads up! This PR modifies the following files:

• @bholley: components/style/media_queries.rs
• @KiChjang: components/script/dom/htmlimageelement.rs

[wafflespeanut]

r? @jdm

[highfive]

New code was committed to pull request.

[jdm]

Length:parse_non_negative does not work for the value calc(100%-3em). Works for __em

This is intentional - the specification says that percentages are not allowed in the sizes calculations.

MediaQuery::parse does not recognize the following inputs:

This is less intentional, but is a result of us using inadequate tools for the job. It turns out that Expression::parse does not parse logical operators like not, but MediaQuery::parse is too broad and parses things that are not valid for the <media-condition> that the specification requires. We should continue to use MediaQuery::parse and leave a TODO comment about replacing it with a more appropriate parser.

[highfive]

New code was committed to pull request.

[jdm]

This is looking really good! There will definitely need to be more unit tests demonstrating correctness before this can merge, however. In addition to verifying the length of the result, comparing each entry in the vector with an expected value would be highly valuable. Tests for single values, values with trailing whitespace, values that contain parse errors, empty values, calling the algorithm with a maximum width, etc.

[highfive]

New code was committed to pull request.

[highfive]

New code was committed to pull request.

[highfive]

New code was committed to pull request.

[sneha1302]

@jdm : Have added a few test cases .
The tidy-test for that doesnot pass as it gives error (./tests/unit/script/lib.rs:22: missing space before -
) which is due to the nature of the

input ("(min-width: 900px) 1000px,
(max-width: 900px) and (min-width: 400px) 50em,
100vw ")
without space after and before "-" (dash))

could you see if the test cases are fine ?

[highfive]

New code was committed to pull request.

[highfive]

New code was committed to pull request.

[highfive]

New code was committed to pull request.

[jdm]

Those tests are a good start; thanks for writing them! A few points:

• it's unusual to see if statements in tests like that; we should be able to make assumptions about the expected output and write tests against those expectations instead.
• all three tests are testing the same input (with the exception of some whitespace changes), which doesn't tell us much about other possible inputs
• there's no coverage for exceptional cases like empty strings, parse errors, etc.

[bors-servo]

☔ The latest upstream changes (presumably #11034) made this pull request unmergeable. Please resolve the merge conflicts.

[jdm]

I've noted this work as part of #11416. Thank you for doing it; we have learned more about what is required for implementing this feature. The lack of tests means that this isn't appropriate to merge right now; we'll return to it as part of #11416.