SVG Painting: computed property values #14211

ewilligers · 2018-11-23T12:30:01Z

Keyword values are as specified.
Lengths are converted to absolute lengths.
Urls are converted to absolute paths.

Found bug:
https://bugs.chromium.org/p/chromium/issues/detail?id=908058
*-opacity should be clamped to [0.1]

AmeliaBR · 2018-12-03T19:28:16Z

Do all browsers downcase keyword values? (e.g., srgb instead of sRGB) I know they are case-insensitive, but I'd expect to use the canonical spelling in the spec. Or maybe change the spec to match, unless there's guidance in a CSS spec about computed format of keywords.

AmeliaBR · 2018-12-03T19:30:32Z

svg/painting/parsing/fill-computed.svg

+  const target = document.getElementById('target');
+  target.style['fill'] = 'url("a.b#c")';
+  const result = getComputedStyle(target)['fill'];
+  assert_true(result.includes('/a.b#c'), result + ' is absolute');


Is there a way to test this more precisely?

Also: there should be a test of bare hash urls (fill="url(#x)").

Advance warning: that's not currently consistently handled by browsers.

let resolved = new URL("a.b#c", document.URL).href;
assert_equals('url("' + resolved + '")', result);

or so ought to work. (Or use HTMLAnchorElement.href, but URL is probably supported widely enough by now.)

Changed to use URL

AmeliaBR · 2018-12-03T19:33:36Z

svg/painting/parsing/stroke-computed.svg

+
+test_computed_value("stroke", 'url("https://example.com/")');
+test_computed_value("stroke", 'url("https://example.com/") none');
+test_computed_value("stroke", 'url("https://example.com/") rgb(12, 34, 56)');


Might want to test (here and for fill) that the color url() values get swapped into a canonical order.

I think values may only occur in one order - there is no || in the <paint> grammar.

The stroke-invalid.svg and fill-invalid.svg tests reject invalid orders.

Interesting. I'd never tested it. Another case where SVG rules aren't following modern CSS practices. But I'm not going to argue for changing it at this point.

AmeliaBR · 2018-12-03T19:37:34Z

svg/painting/parsing/stroke-dasharray-computed.svg

+test_computed_value("stroke-dasharray", "calc(50% + 60px)");
+
+test_computed_value("stroke-dasharray", "10px, 20%, 30px");
+test_computed_value("stroke-dasharray", "0, 5", "0px, 5px");


Do we have a canonical rule about using comma separators vs space separators?

(I would have expected space to be the canonical version, myself.) Either way, we should test both.

https://drafts.csswg.org/css-values-4/#component-multipliers

A hash mark (#) indicates that the preceding type, word, or group occurs one or more times, separated by comma tokens

https://drafts.csswg.org/cssom/#common-serializing-idioms

To serialize a comma-separated list concatenate all items of the list in list order while separating them by ", ", i.e., COMMA (U+002C) followed by a single SPACE (U+0020).

I have added a test to stroke-dasharray-computed.svg (this PR) and stroke-dasharray-valid.svg (#14367)

It's a bit more complicated than that with SVG, because most properties allow either comma or space separation, with no distinction in meaning. SVG 2 uses the <value>#* combination list operator to describe it (a space-separated list of comma-separated lists), but that is slightly abusing the CSS syntax.

In this particular case, comma separation makes sense since it's an arbitrary-length list of similar values. And all browsers I tested did it that way, anyway. If we ever make viewBox or points or other attributes into CSS properties, then we'll have to deal with whether we want to enforce logic on the list separators!

Thanks for adding the extra test for space-separated values.

AmeliaBR · 2018-12-03T19:40:06Z

svg/painting/parsing/stroke-miterlimit-computed.svg

+test_computed_value("stroke-miterlimit", "0");
+test_computed_value("stroke-miterlimit", "0.5");
+test_computed_value("stroke-miterlimit", "1");
+test_computed_value("stroke-miterlimit", "7.5");


Can this particular testing framework test correct handling of invalid values? E.g., a negative value in this case should be rejected by the parser.

Invalid values are tested in */parsing/*-invalid.svg; these found browser bugs relating to image-rendering, stroke-opacity, stroke-dasharray, stroke-width and more.

stroke-miterlimit-invalid.svg tests that negative values and misformed numbers are rejected.

AmeliaBR · 2018-12-03T19:43:44Z

Another general comment:

The testing of length values should include more units, including CSS3 units.

Of course, confirming that the CSS systems correctly handle those units doesn't mean that the rendering system will handle them correctly (test what some of the calc() values render as, to find out what I mean). But it's a start.

ewilligers · 2018-12-04T23:42:25Z

Do all browsers downcase keyword values

Firefox and Chrome Canary downcase, consistent with the spec Serializing CSS Values

To serialize a CSS component value depends on the component, as follows:
keyword

The keyword converted to ASCII lowercase.

There are open bugs for WebKit and Edge.

I'd expect to use the canonical spelling in the spec.

I'd also be happy for the spec to use lower case.

We have free choice: the Values and Units spec has Hz, kHz and Q, and all of the following CSS spec files contain both currentColor and currentcolor:

css-backgrounds-3/Overview.bs
css-color-3/Overview.html
css-color-3/Overview.src.html
css-color-4/Overview.bs
css-pseudo-4/Overview.bs
css-text-decor-3/Overview.bs
css-ui-4/Overview.bs
css-values-4/Overview.bs

AmeliaBR · 2018-12-05T01:25:38Z

We have free choice: the Values and Units spec has Hz, kHz and Q, and all of the following CSS spec files contain both currentColor and currentcolor:

OK, then let's leave it as is. The mixed case versions are more readable, but so long as there is a clear spec that says the lowercase versions should be used for serialization. If it was a new CSS spec, we'd use hyphenated words, but too late for that, now.

Keyword values are as specified. Lengths are converted to absolute lengths. Urls are converted to absolute paths. Found bug: https://bugs.chromium.org/p/chromium/issues/detail?id=908058 *-opacity should be clamped to [0.1]

ewilligers · 2018-12-05T12:08:37Z

I think all comments have been addressed now, in either this PR or #14367.

ewilligers added the svg label Nov 23, 2018

ewilligers requested review from dirkschulze and AmeliaBR November 23, 2018 12:30

wpt-pr-bot added the wg-svg label Nov 23, 2018

wpt-pr-bot assigned heycam Nov 23, 2018

wpt-pr-bot requested review from heycam, nikosandronikos and svgeesus November 23, 2018 12:30

AmeliaBR reviewed Dec 3, 2018

View reviewed changes

wpt-pr-bot requested a review from AmeliaBR December 3, 2018 19:30

AmeliaBR reviewed Dec 3, 2018

View reviewed changes

wpt-pr-bot requested a review from AmeliaBR December 3, 2018 19:33

AmeliaBR reviewed Dec 3, 2018

View reviewed changes

wpt-pr-bot requested a review from AmeliaBR December 3, 2018 19:37

AmeliaBR reviewed Dec 3, 2018

View reviewed changes

wpt-pr-bot requested a review from AmeliaBR December 3, 2018 19:40

ewilligers mentioned this pull request Dec 4, 2018

Browsers do not accept stroke-linejoin 'miter-clip' or 'arcs' w3c/svgwg#592

Closed

ericwilligers added 2 commits December 5, 2018 20:45

SVG Painting: computed property values

800ed86

Keyword values are as specified. Lengths are converted to absolute lengths. Urls are converted to absolute paths. Found bug: https://bugs.chromium.org/p/chromium/issues/detail?id=908058 *-opacity should be clamped to [0.1]

SVG Painting: length units and computed style

87b7fb4

ewilligers force-pushed the painting-computed branch from d1eb774 to 87b7fb4 Compare December 5, 2018 11:19

Test resolved URL

1b0b868

AmeliaBR approved these changes Dec 5, 2018

View reviewed changes

ewilligers merged commit 5781139 into web-platform-tests:master Dec 5, 2018

ewilligers deleted the painting-computed branch December 5, 2018 20:06

jugglinmike mentioned this pull request Aug 21, 2019

Redundant review requests in response to reviews web-platform-tests/wpt-pr-bot#75

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SVG Painting: computed property values #14211

SVG Painting: computed property values #14211

ewilligers commented Nov 23, 2018

AmeliaBR commented Dec 3, 2018

AmeliaBR Dec 3, 2018

AmeliaBR Dec 3, 2018

fsoder Dec 3, 2018

ewilligers Dec 5, 2018

AmeliaBR Dec 3, 2018

ewilligers Dec 5, 2018

AmeliaBR Dec 5, 2018

AmeliaBR Dec 3, 2018

ewilligers Dec 5, 2018

AmeliaBR Dec 5, 2018

AmeliaBR Dec 3, 2018

ewilligers Dec 5, 2018

AmeliaBR commented Dec 3, 2018

ewilligers commented Dec 4, 2018

AmeliaBR commented Dec 5, 2018

ewilligers commented Dec 5, 2018

SVG Painting: computed property values #14211

SVG Painting: computed property values #14211

Conversation

ewilligers commented Nov 23, 2018

AmeliaBR commented Dec 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmeliaBR commented Dec 3, 2018

ewilligers commented Dec 4, 2018

AmeliaBR commented Dec 5, 2018

ewilligers commented Dec 5, 2018