w3c / svgwg

SVG Working Group specifications
Other
708 stars 133 forks source link

'd' presentation attribute inconsistent with shipped implementation and offset-path #320

Open ewilligers opened 7 years ago

ewilligers commented 7 years ago

Blink shipped the d presentation attribute with value: path() | none based on the Sydney face to face discussions early in 2016, and the spec text from around the same time.

This is consistent with the syntax for offset-path https://drafts.fxtf.org/motion-1/#offset-path-property

I didn't realize the syntax had changed in a teleconference https://github.com/w3c/svgwg/issues/119 until https://crbug.com/652822 was raised after Blink shipped.

It isn't possible to change the syntax returned by getComputedStyle in a backwards compatible way.

Please consider restoring the spec syntax to path() | none

This has two major advantages:

AmeliaBR commented 7 years ago

There were extensive discussions about this, which no one from Blink participated in. See #49 and #119. The version shipped by Blink does not make sense & is inconsistent with other properties. The fact that it was shipped based on an early draft that was still being debated in the working group, with no experimental flag, is something that reflects poorly on Blink's implementation process.

The path() function, as spec'd, is a <shape-function> data type that can be used and replaced in any context where circle(), ellipse(), polygon() and inset() are used. Furthermore, the path() function includes a fill-rule parameter, but the fill-rule for <path> elements is defined by a separate property.

In contrast, offset-path, as spec'd, should accept any of the shape functions. (The fill-rule isn't relevant, but it isn't a contradiction, either.) Last I checked, Blink's motion path implementation only accepted path, but that is a limitation of the implementation.

fsoder commented 7 years ago

I'm not sure I agree with "does not make sense & is inconsistent" - you can't get from an attribute value to a <string> without massaging/preprocessing the former (adding quotes, escaping newlines.) That path()is a <basic-shape> also doesn't restrict it from appearing in other productions. Choosing "untyped" over "typed" is what doesn't make sense to me. That's pretty irrelevant though, because we can "fix" (recognize and serialize <string>) this in Blink (I'll comment on the bug mentioned.) The ship is ways out to sea though, and I'm not sure d: <string> is going to outsell d: <path()>. I guess the least we can do is try though, with use counters and whatnot ('d' property usage is already tallied, but not the value set), even if that could just appear to be a play to the gallery.

ewilligers commented 7 years ago

Note that fill-rule was removed from the path function (proposal, motion path issue).

The syntax of the 'd' presentation attribute in the spec, before the path function was removed, was none | path( \<string> )

shans commented 7 years ago

The fact that it was shipped based on an early draft that was still being debated in the working group, with no experimental flag, is something that reflects poorly on Blink's implementation process.

I want to make it very clear for the record how ridiculous this statement sounds. At the 2016 face-to-face meeting which Eric references, the working group spent a considerable amount of time asking browser vendors to implement and ship more of SVG 2. Are we now to be taken to task for fulfilling the working group's wishes?

I don't find the SVGWG's reasons for dropping the path() function to be very compelling. While it could be viewed as technically being consistent for the d property to differ from consumers of <basic-shape>, this is a distinction that will be lost on most authors. It's far more usable to accept the path() function everywhere, even if SVG never accepts the other <basic-shape>s.

nikosandronikos commented 7 years ago

I want to make it very clear for the record how ridiculous this statement sounds. At the 2016 face-to-face meeting which Eric references, the working group spent a considerable amount of time asking browser vendors to implement and ship more of SVG 2. Are we now to be taken to task for fulfilling the working group's wishes?

While there were certainly requests for implementation of SVG2 features, to be fair, there was also a process where the group would resolve on what features were ready for implementation and I don't recall a resolution for this feature.

shans commented 7 years ago

The minutes (https://www.w3.org/2016/02/05-svg-irc) seem to reflect that there was talk about setting up such a process in April. Critically, this was intended to help implementers who were paralyzed by choice, not to prohibit implementers from implementing bits.

The flavor of those minutes is very much "please implement more stuff".

alexjlockwood commented 6 years ago

This is WAI according to this bug: https://bugs.chromium.org/p/chromium/issues/detail?id=652822#c8

svgeesus commented 6 years ago

Adding Agenda+ because it is not clear fro reading the issue which way we should go here. I note that @ewilligers has a patch ready which would fix this, assuming we accept the path() function which I think makes sense from a CSS consistency viewpoint.

css-meeting-bot commented 6 years ago

The Working Group just discussed d property value, and agreed to the following:

The full IRC log of that discussion <krit> topic: d property value
<krit> GitHub: https://github.com/w3c/svgwg/issues/320
<krit> krit: So we only support the path CSS function and the none keyword on the CSS property but not the path string directly?
<krit> ericwilligers: that is correct.
<krit> Tav: we treat the attribute and property the same. So for us this is difficult.
<krit> ericwilligers: but we still support the old syntax on the attribute.
<krit> ericwilligers: what is the implementation issue?
<krit> krit: WebKit and Blink usually use the same parser for the attribute and property on presentation attributes.
<krit> AmeliaBR: there are some difference in implementations already like unit less length value support for attributes and properties.
<krit> krit: but this is just a flag in the cSS parser for implementations.
<krit> AmeliaBR: then we have the difference on transform attribute.
<krit> krit: here again... there is the problem of how transforms get stored internally. It works for browsers but maybe not for d property. Maybe it does though.
<krit> AmeliaBR: my concern is mostly about the different flavour of the CSS shape function.
<krit> krit: I think there are 2 issues... 1) there is the fill-rule value as part of path() shape function. and 2) the missing other shape functions.
<krit> ericwilligers: that is right, that is missing.
<krit> AmeliaBR: yes, that is another concern.
<krit> krit: do we think we can spec something in SVG2 or should we put it in other spec?
<krit> AmeliaBR: implementations just don't know what to do. Otherwise they would do it.
<krit> Tav: We implemented the d property read-only
<krit> krit: with the CSS function path?
<krit> Tav: yes and that is reasonly because we treat attributes and properties the same way.
<krit> AmeliaBR: one option would be to extend the syntax of the attribute.
<ericwilligers> Custom property use case: https://jsfiddle.net/ericwilligers/jq2af341/
<krit> krit: I wanted to get to this as well... should we add everything from the property to the attribute as well?
<krit> krit: lets start with the discussion if we want to defer or if we can finalise for SVG 2
<krit> AmeliaBR: my concern is as longer as the Chrome implementation is out the less likely the implementation will change
<krit> krit: I share the concern and we already have this with clip-path and the outdated implementation in Blink.
<krit> krit: I'd like to have a conclusion between the WG members.
<krit> AmeliaBR: Tav, what do you do about CSS transforms?
<krit> Tav: we don't implement it yet.
<krit> krit: Can you live with just starting work on the path function for now AmeliaBR ?
<krit> AmeliaBR: if that is the choice between deferring and only path yes.
<krit> AmeliaBR: I'd be happier with a living long term plan to support all shape function.
<krit> krit: I agree but the other shape functions are harder to specify because of the relative values they support.
<krit> AmeliaBR: path() uses absolute units but the other shapes do support percentages and should be relative to boxes. I don't want to lock d on path() only.
<krit> krit: I'd like us to focus on path() only for now and discuss boxes and other shapes later.
<krit> RESOLUTION: d property only supports path() for now but potentially will support other CSS shape function
<krit> AmeliaBR: 2nd resolution would be to decide if d attribute only supports string or will support shapes as well in the future.
<krit> ericwilligers: I'd advocate to let d attribute only support path string and the property to support shape functions and no string
<krit> krit: the decision to only support string on the attribute does not block us to add functions later
<krit> proposed RESOLUTION: The d property will support the shape function path() only for now. The d attribute will only support SVG path string an no functional notation. d will be a presentation attribute and contributes into the CSS styling hierarchy as other presentation attributes.
<krit> RESOLUTION: The d property will support the shape function path() only for now. The d attribute will only support SVG path string an no functional notation. d will be a presentation attribute and contributes into the CSS styling hierarchy as other presentation attributes.
<krit> krit: last question from me: Can we align path() to the CSS Shape functions??
<krit> AmeliaBR: we could have one data type path() function
<AmeliaBR> <path-function> = path("..svg path code..")
<AmeliaBR> <shape-function>=<path-function> | ellipse(..) | circle(..) | ...
<krit> krit: IMO it is still confusing for content creators why we have patch() w/ fill rule and one path() w/o fill rule setting? We would just need to clarify how the fill rule interacts with the fill-rule/clip-rule properties?
<krit> AmeliaBR: My suggestion was to have 3 options for fill-rule... default w/o fill rule would act as "auto" value... which would be fill-rule/clip-rule property on the d property. But with explicit fill rule to would overwrite the properties.
<krit> AmeliaBR: default would be current behavior and with the explicit value the implementation would use this explicit value
<krit> https://drafts.fxtf.org/motion-1/#offsetpath-pathfunc
<krit> ericwilligers: it was removed as value since it is not needed for offset-path
<krit> AmeliaBR: but it would be needed for clip-path and shape properties.
<krit> AmeliaBR: currently the polygon() function has default values for missing winding rules. We would need to clarify with the CSS WG if we can make the default value dependent not he CSS property using the function notation...
<krit> AmeliaBR: the same would apply to path()
<krit> AmeliaBR: define that the missing value does not compute until used value time.
<AmeliaBR> s/not he/on the/
<krit> krit: could you bring this up to the CSS wG?
<krit> AmeliaBR: I could
<krit> ericwilligers: what is the motivation
<AmeliaBR> > Computed Values of Basic Shapes https://drafts.csswg.org/css-shapes/#basic-shape-computed-values
<AmeliaBR> Omitted values are included and compute to their defaults.
<krit> krit: any resolutions we need to add still?
<krit> RESOLUTION: Discuss winding rule issue of polygon() and path() with CSS WG. Especially with regards of use in different properties like offset-path, d and clip-path.
<krit> RESOLUTION: Keep path() without fill rule on d property for now.
<krit> trackbot, end telcon