make transform="" a presentation attribute for the transform property

ASSIGNED
Assigned to

Status

()

Core
SVG
ASSIGNED
5 years ago
a day ago

People

(Reporter: heycam, Assigned: louis)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments)

In SVG 2, the trasnform="" attribute has become a presentation attribute for the transform property.

https://svgwg.org/svg2-draft/coords.html#TransformProperty
(Assignee)

Updated

5 months ago
Duplicate of this bug: 1381201
(Assignee)

Comment 2

5 months ago
Created attachment 8890728 [details]
svg-transform.svg
(Assignee)

Comment 3

5 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=09b2b66f124c65b4beaee8f82a444ac98296a02b
(Assignee)

Comment 4

5 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=41e66d9a0c6199359d67834f83301cfa9710ff89
(Assignee)

Comment 5

5 months ago
Created attachment 8891862 [details] [diff] [review]
[WIP] Bug 878346 - make transform="" a presentation attribute for the transform property
Assignee: nobody → lochang
Status: NEW → ASSIGNED
(Assignee)

Comment 6

5 months ago
Created attachment 8891868 [details]
additive-1.svg
(Assignee)

Comment 7

5 months ago
The patch in comment 5 seems to fix the test case svg-transform.svg but also breaks some animation reftests (see try results in comment 4). And I simplify one of the test cases additive-1.svg, and seems that transform translate does work but I have no idea why the animation is broken...
(Assignee)

Comment 8

5 months ago
Hi Brian,
Flags: needinfo?(bbirtles)
(Assignee)

Comment 9

5 months ago
(In reply to Louis Chang [:louis] from comment #8)
> Hi Brian,

Seems you are working on the animation, could you help me out with the problem mentioned in comment 7?
(Assignee)

Comment 10

5 months ago
(In reply to Louis Chang [:louis] from comment #7)
> The patch in comment 5 seems to fix the test case svg-transform.svg but also
> breaks some animation reftests (see try results in comment 4). And I
> simplify one of the test cases additive-1.svg, and seems that transform
> translate does work but I have no idea why the animation is broken...

Note that if I remove the transform="translate(50)" in path element, the animation works again.
(In reply to Louis Chang [:louis] from comment #7)
> The patch in comment 5 seems to fix the test case svg-transform.svg but also
> breaks some animation reftests (see try results in comment 4). And I
> simplify one of the test cases additive-1.svg, and seems that transform
> translate does work but I have no idea why the animation is broken...

I haven't looked into this in any detail, but I imagine promoting `transform` to a presentation attribute involves converting it to a mapped attribute including:

* Adding it to SVGTransformableElement::IsAttributeMapped
* Making the SVGAnimatedTransformList's animVal and baseVal attributes look/modify up the mapped-attribute style.
  animVal should technically only return the SMIL-animated style, but I suppose it would be ok to make it equivalent to `getComputedStyle(elem).transform`. baseVal should return the style specified on the attribute (as opposed to simply the un-animated style).
  We'll possibly deprecate animVal at some time but we really should do that for all interfaces at once, not piecemeal. I notice SVG2 still specifies it.
* Making <animateTransform> update the corresponding mapped-attribute style.
* Removing some of the special handling for SVG transforms (see uses of nsIFrame::IsSVGTransformed in graphics and animation code)
Flags: needinfo?(bbirtles)
(In reply to Brian Birtles (:birtles) from comment #11)
>   We'll possibly deprecate animVal at some time but we really should do that
> for all interfaces at once, not piecemeal. I notice SVG2 still specifies it.
> * Making <animateTransform> update the corresponding mapped-attribute style.

The current SVG 2 spec text says that the SVGAnimatedTransformList's baseVal and animVal should reflect the presentation attribute value.  That is part of the wider decision to get rid of animVal's functionality, but with lesser breakage risk than removing the animVal property altogether.  If we did that, I think it would simplify the work in this bug, since we wouldn't have to worry about correctly exposing and updating a DOMSVGTransformList to script that represents the current computed value of the transform property.

So what do you think about doing that first?  Do you know what the status of animVal is other browsers?
Flags: needinfo?(bbirtles)
That sounds ok to me. I don't know what the status is of other browsers. (My impression is no-one's really touching their SVG code but, like us, they'll make these changes when they're in the neighborhood.)
Flags: needinfo?(bbirtles)
(Assignee)

Comment 14

4 months ago
I will not work on this bug for now since it's out of my knowledge...
Feel free to take over the bug if anyone want to solve it.
Blocks: 779685
(Assignee)

Comment 15

2 months ago
Created attachment 8918125 [details]
ref_testcase
You need to log in before you can comment on or make changes to this bug.