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.

Sign up for GitHub

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

Jump to bottom

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

Open
ewilligers opened this issue May 27, 2017 · 9 comments · May be fixed by #374
Open

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

ewilligers opened this issue May 27, 2017 · 9 comments · May be fixed by #374
Assignees
@ewilligers
Labels
CSS Consistency Paths chapter SVG Paths SVG2
Milestone
SVG 2.0 Proposed ...

Comments

@ewilligers
Copy link
Contributor

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
#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:

  • consistency with the only shipping implementation
  • consistency with offset-path
@ewilligers ewilligers mentioned this issue May 27, 2017
Open
@AmeliaBR
Copy link
Contributor

AmeliaBR commented May 27, 2017 •
edited
Loading

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
Copy link

fsoder commented May 27, 2017

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
Copy link
Contributor Author

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
Copy link
Contributor

shans commented May 29, 2017

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
Copy link
Member

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
Copy link
Contributor

shans commented May 31, 2017 •
edited
Loading

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".

@ewilligers ewilligers mentioned this issue Sep 25, 2017
Closed
@alexjlockwood
Copy link

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

ewilligers pushed a commit to ewilligers/svgwg that referenced this issue Dec 18, 2017
@ericwilligers
'd' presentation attribute: use path() function
25c0cd2 
This brings the spec, the web platform tests and the only
shipping implementation (Chrome) into consistency:

  none | path(<string>)

closes w3c#320
@ewilligers ewilligers linked a pull request Dec 18, 2017 that will close this issue
Open
ewilligers pushed a commit to ewilligers/svgwg that referenced this issue Dec 19, 2017
@ericwilligers
'd' presentation attribute: use path() function
71900f5 
This brings the spec, the web platform tests and the only
shipping implementation (Chrome) into consistency:

  none | path(<string>)

closes w3c#320
@boggydigital boggydigital added this to the SVG 2.0 Recommendation milestone Jun 11, 2018
@ewilligers ewilligers self-assigned this Jun 25, 2018
@svgeesus
Copy link
Contributor

svgeesus commented Aug 5, 2018

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.

ewilligers pushed a commit to ewilligers/svgwg that referenced this issue Aug 14, 2018
@ericwilligers
'd' presentation attribute: use path() function
6bfd181 
This brings the spec, the web platform tests and the only
shipping implementation (Chrome) into consistency:

  none | path(<string>)

closes w3c#320
@css-meeting-bot
Copy link
Member

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

  • RESOLUTION: d property only supports path() for now but potentially will support other CSS shape function
  • 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.
  • 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.
  • RESOLUTION: Keep path() without fill rule on d property for now.
The full IRC log of that discussion <krit> topic: d property value
<krit> GitHub: https://github.com//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

ewilligers pushed a commit to ewilligers/svgwg that referenced this issue Nov 5, 2018
@ericwilligers
'd' presentation attribute: use path() function
3827d13 
This brings the spec, the web platform tests and the only
shipping implementation (Chrome) into consistency:

  none | path(<string>)

closes w3c#320
SimonSapin added a commit to SimonSapin/svgwg that referenced this issue Dec 27, 2018
@SimonSapin
<path d> parsing spec needs a special case to not use CSS syntax
5e0ef74 
When reading the spec literally before this changes,
https://svgwg.org/svg2-draft/styling.html#PresentationAttributes
specifies that all presentation properties are parsed according to
https://svgwg.org/svg2-draft/types.html#presentation-attribute-css-value

For the `d` property, this means the CSS grammar `none | <string>`.

The following would be valid:

* `<path d="none">`
* `<path d="'M 100 100 L 300 100 L 200 300 z'">` (not the single quotes)
* `<path d="/**/'M 100 100 \L \3300 100 L 200 300 z'">` (equivalent to the previous)

But `<path d="M 100 100 L 300 100 L 200 300 z">` or indeed any SVG 1.1 path
would not be valid because they parse as CSS ident and number tokens,
not as a `<string>`.

w3c#320 poposes changing the syntax
of the `d` CSS propery to be even further to the existing attribute syntax.
SimonSapin added a commit to SimonSapin/svgwg that referenced this issue Dec 27, 2018
@SimonSapin
<path d> parsing spec needs a special case to not use CSS syntax
2412b07 
When reading the spec literally before this changes,
https://svgwg.org/svg2-draft/styling.html#PresentationAttributes
specifies that all presentation properties are parsed according to
https://svgwg.org/svg2-draft/types.html#presentation-attribute-css-value

For the `d` property, this means the CSS grammar `none | <string>`.

The following would be valid:

* `<path d="none">`
* `<path d="'M 100 100 L 300 100 L 200 300 z'">` (not the single quotes)
* `<path d="/**/'M 100 100 \L \33 00 100 L 200 300 z'">` (equivalent to the previous)

But `<path d="M 100 100 L 300 100 L 200 300 z">` or indeed any SVG 1.1 path
would not be valid because they parse as CSS ident and number tokens,
not as a `<string>`.

w3c#320 poposes changing the syntax
of the `d` CSS propery to be even further to the existing attribute syntax.
SimonSapin added a commit to SimonSapin/svgwg that referenced this issue Dec 27, 2018
@SimonSapin
<path d> parsing spec needs a special case to not use CSS syntax
125c975 
When reading the spec literally before this changes,
https://svgwg.org/svg2-draft/styling.html#PresentationAttributes
specifies that all presentation properties are parsed according to
https://svgwg.org/svg2-draft/types.html#presentation-attribute-css-value

For the `d` property, this means the CSS grammar `none | <string>`.

The following would be valid:

* `<path d="none">`
* `<path d="'M 100 100 L 300 100 L 200 300 z'">` (not the single quotes)
* `<path d="/**/'M 100 100 \L \33 00 100 L 200 300 z'">` (equivalent to the previous)

But `<path d="M 100 100 L 300 100 L 200 300 z">` or indeed any SVG 1.1 path
would not be valid because they parse as CSS ident and number tokens,
not as a `<string>`.

w3c#320 poposes changing the syntax
of the `d` CSS propery to be even further to the 1.1 attribute syntax.

This does not appear to be an intentional change from SVG 1.1,
so this pull request "reverts" it.
@SimonSapin SimonSapin mentioned this issue Dec 27, 2018
Closed
SimonSapin added a commit to SimonSapin/svgwg that referenced this issue Dec 27, 2018
@SimonSapin
<path d> parsing spec needs a special case to not use CSS syntax
18aee92 
When reading the spec literally before this changes,
https://svgwg.org/svg2-draft/styling.html#PresentationAttributes
specifies that all presentation properties are parsed according to
https://svgwg.org/svg2-draft/types.html#presentation-attribute-css-value

For the `d` property, this means the CSS grammar `none | <string>`.

The following would be valid:

* `<path d="none">`
* `<path d="'M 100 100 L 300 100 L 200 300 z'">` (not the single quotes)
* `<path d="/**/'M 100 100 \L \33 00 100 L 200 300 z'">` (equivalent to the previous)

But `<path d="M 100 100 L 300 100 L 200 300 z">` or indeed any SVG 1.1 path
would not be valid because they parse as CSS ident and number tokens,
not as a `<string>`.

w3c#320 poposes changing the syntax
of the `d` CSS propery to be even further to the 1.1 attribute syntax.

This does not appear to be an intentional change from SVG 1.1,
so this pull request "reverts" it.
@karlcow karlcow mentioned this issue Jun 11, 2020
Closed
@faceless2 faceless2 mentioned this issue Dec 2, 2020
Merged
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jun 10, 2021
@BorisChiou
Bug 1340422 - Part 1: Add SVG d property in CSS. r=emilio
d61e621 
Add d property for style system. d property only supports path() for now
and it has the functional notation without fill rule.

w3c/svgwg#320 (comment)

Differential Revision: https://phabricator.services.mozilla.com/D81237
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Jun 15, 2021
@BorisChiou
Bug 1340422 - Part 1: Add SVG d property in CSS. r=emilio
3643e6f 
Add d property for style system. d property only supports path() for now
and it has the functional notation without fill rule.

w3c/svgwg#320 (comment)

Differential Revision: https://phabricator.services.mozilla.com/D81237
@anup-a anup-a mentioned this issue Nov 21, 2021
Closed
Loirooriol added a commit to Loirooriol/servo that referenced this issue May 22, 2023
@Loirooriol
style: Part 1: Add SVG d property in CSS
488338e 
Add d property for style system. d property only supports path() for now
and it has the functional notation without fill rule.

w3c/svgwg#320 (comment)

Differential Revision: https://phabricator.services.mozilla.com/D81237
Loirooriol pushed a commit to Loirooriol/servo that referenced this issue May 24, 2023
@BorisChiou @Loirooriol
style: Part 1: Add SVG d property in CSS
8d783f3 
Add d property for style system. d property only supports path() for now
and it has the functional notation without fill rule.

w3c/svgwg#320 (comment)

Differential Revision: https://phabricator.services.mozilla.com/D81237
Loirooriol pushed a commit to Loirooriol/servo that referenced this issue May 24, 2023
@BorisChiou @Loirooriol
style: Part 1: Add SVG d property in CSS
fa6ab91 
Add d property for style system. d property only supports path() for now
and it has the functional notation without fill rule.

w3c/svgwg#320 (comment)

Differential Revision: https://phabricator.services.mozilla.com/D81237
cdoublev added a commit to cdoublev/css that referenced this issue Oct 11, 2024
@cdoublev
Fix value definition of the "d" property
b484427 
Based on w3c/svgwg#320 and most if not all browser implementations.
cdoublev added a commit to cdoublev/css that referenced this issue Oct 11, 2024
@cdoublev
Fix value definition of the "d" property
035da2e 
Based on w3c/svgwg#320 and most if not all browser implementations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ewilligers ewilligers

Labels
CSS Consistency Paths chapter SVG Paths SVG2
Projects
None yet
Milestone
SVG 2.0 Proposed Recommendation
Development

Successfully merging a pull request may close this issue.

'd' presentation attribute: use path() function ewilligers/svgwg
9 participants
@boggydigital @shans @ewilligers @alexjlockwood @svgeesus @nikosandronikos @AmeliaBR @fsoder @css-meeting-bot