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.

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

[css-transforms-1] Clarify behavior for gradientTransform and patternTransform #919

Closed
smfr opened this issue Jan 13, 2017 · 8 comments
Closed

Comments

@smfr
Copy link
Contributor

smfr commented Jan 13, 2017

https://www.w3.org/Bugs/Public/show_bug.cgi?id=26470

@smfr smfr changed the title Clarify behavior for gradientTransform and patternTransform [css-transforms-1] Clarify behavior for gradientTransform and patternTransform Jan 13, 2017
@smfr smfr added the SVG label Jan 14, 2017
@smfr
Copy link
Contributor Author

smfr commented Nov 6, 2017

Erik said:

I found http://www.w3.org/TR/css3-transforms/#svg-gradient-transform-pattern-transform slightly confusing, as normally presentation attributes can be specified on any svg element (see https://svgwg.org/svg2-draft/intro.html#TermPresentationAttribute). It seems the desire here is to break that convention, and regardless of the intention the behavior needs to be specified - to explicitly call out the difference(s) to other presentation attributes AND/OR to specify the order in which transform, patternTransform and gradientTransform are mapped if they're all specified on the same element.

It's possible that the SVG2 spec will need to be updated as well, see https://svgwg.org/svg2-draft/styling.html#UsingPresentationAttributes, since with the section in the Transforms spec a given presentation attribute and the corresponding property may no longer share the same name.

@smfr
Copy link
Contributor Author

smfr commented Nov 9, 2017

Question: should work? ChrisL suggests that the transform spec say MAY or SHOULD for this (no browser implements this AFAIK).

@smfr
Copy link
Contributor Author

smfr commented Nov 9, 2017

SVG 2 says this should work: https://svgwg.org/svg2-draft/styling.html#PresentationAttributes

@dirkschulze
Copy link
Contributor

I am unsure about the idea that transform, patternTransform and gradientTransform can be applied on the same element. The SVG spec defines which presentation attribute can be defined per element. We could simply revert the part

Any element in the SVG namespace; however, for historical reasons the ‘linearGradient’ and ‘radialGradient’ elements use the attribute name ‘gradientTransform’, and the ‘pattern’ element uses ‘patternTransform’.

in SVG2 to SVG 1.1's behavior. That would make more sense anyway.

@AmeliaBR
Copy link
Contributor

AmeliaBR commented Jan 8, 2018

In hindsight, the SVG 2 text was not as clear as it could be. The intention (as I understood it) was that a transform attribute would have no effect on linearGradient, radialGradient, and pattern. However, the transform style property would still have an effect, and the legacy attributes would be presentation attributes for it.

If the long-term goal is to drop the legacy attributes and allow the transform attribute in their place, then yes, we would need clear precedence rules for when they are both specified on the same element.

@dirkschulze
Copy link
Contributor

@AmeliaBR If I recover correctly, transform on <pattern> and <*Gradient> get ignored by implementations today. I suggest that we keep implemented behavior and do not specify transform as a presentation attribute for patterns and gradients. Then we do not have to specify precedence rules and are conform with implementations.

It might be that for implementations it is easier if the transform attribute works for gradients and patterns so that they can more easily map. IMO an implementation detail.

@dirkschulze
Copy link
Contributor

dirkschulze commented Jan 8, 2018

I wrote a simple test here: https://codepen.io/krit/pen/zppKNN

It seems like Chrome Canary already supports the CSS transform property on patterns and gradients. (Requires detailed testing to be sure.)

The important observation:

  • The transform and gradientTransform presentation attributes do not apply to the pattern element.
  • The transform and patternTransform presentation attributes do not apply to the gradient elements.

I think that this makes sense. Should still be defined in the presentation attribute section and per content model in SVG2 and not in this specification. This specification should just map those attributes to the transform property and it does.

@css-meeting-bot
Copy link
Member

The Working Group just discussed Clarify patternTransform/linearTransform.

The full IRC log of that discussion <Chris__> Topic: Clarify patternTransform/linearTransform
<Chris__> https://github.com/w3c/svgwg/pull/378
<Chris__> scribenick: Chris__
<Chris__> krit: CS transform spec introduces transform property.
<Chris__> ... has a presentation transform attribute
<Chris__> ... problem is with pattern and gradients. did not clarify where these can be specified
<Chris__> ... and order of application
<AmeliaBR> github: https://github.com/w3c/svgwg/pull/378
<Chris__> krit: blink allow on pattern element, onlt
<AmeliaBR> github: https://github.com//issues/919
<Chris__> ... similarly on gradient, but not transform attribute. Much like svg 1.1
<Chris__> ... so these are all for the same transform property
<Chris__> Chris__, so these are aliases to the same property
<Chris__> krit: so content model does not change wrt SVG 1.1
<Chris__> AmeliaBR, that is what we wanted for SVG2 anyway, this is an editoral clarification?
<Chris__> krit: no, currently they are all allowed everywhere. want to rever to 1.1 behaviour
<Chris__> AmeliaBR, ok
<Chris__> Chair: Chris
<Chris__> krit: if they are both on the same element then we need to establish order
<Chris__> AmeliaBR, we had a couple different places related to presentation attribues
<Chris__> krit: the PR specifies this
<AmeliaBR> https://svgwg.org/svg2-draft/styling.html#PresentationAttributes
<Chris__> AmeliaBR, not as clear as it should be
<Chris__> krit: is the PR okay?
<Chris__> AmeliaBR, we need to update the changelog too
<Chris__> Resolved: explanation from Dirk looks good, accept
<krit> scribenick: krit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants