Support IE/Edge SVG transforms #148

dvoytenko · 2017-07-06T04:06:07Z

I'd like to discuss a possible workaround for IE/Edge lack of support of transform styles for SVG elements. E.g. see https://connect.microsoft.com/IE/feedback/details/811744/ie11-bug-with-implementation-of-css-transforms-in-svg and https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/1173754/

As README.md correctly states:

By unifying the animation features of SVG and CSS, Web Animations unlocks features previously only usable declaratively, and exposes powerful, high-performance animation capabilities to developers.

This workaround would essentially polyfill this statement for IE/Edge.

If this change is acceptable, I'll add tests, add more features (e.g. transform-origin) and more docs. PTAL.

Closes #83.

/cc @emarchiori @jasti

dvoytenko · 2017-07-06T16:01:12Z

/cc @fearmear

alancutter · 2017-07-07T05:45:14Z

I'm very happy with the code change here, it's simple, well documented and fits well with existing code.
Tests are required before this can land though, I think it's fine to mock the userAgent string for testing purposes.

I'm concerned about the support implications of this change. If Edge implements the Web Animations API without fixing their SVG transform property behaviour this issue will regress back to the current state.
If we patch over their behaviour this time are we responsible for patching over it again in the future?

An alternative way of resolving this is to create a separate polyfill that adds another wrapper around the inline style object which only does the SVG transform attribute switcheroo. This is probably worse for users and runtime performance/complexity but does maintain a separation of responsibilities in the project. WDYT?

dvoytenko · 2017-07-07T21:39:01Z

@alancutter We were (are) definitely considering style-only shim. It's definitely possible, though gets a bit confusing with transform-origin.

I'm trying to confirm right now if the plan to release web animations on Edge assumes that SVG transforms will be supported as well. I think, however, that if it didn't and it were important for the project to animation SVG elements, the only option would be to force override native Web Animations with the polyfill - it'd amount to native implementation being incomplete.

WDYT?

RByers · 2017-07-11T20:50:35Z

/cc @boggydigital

dvoytenko · 2017-07-11T23:24:32Z

@alancutter Overall, I'm getting signals that SVG transforms would be released no later than Web Animations. So, I think this makes a good case for continuing with Web Animations polyfill to address this. WDYT?

boggydigital · 2017-07-13T00:30:22Z

Sorry I'm late to the discussion, while there is no semantic coupling for Microsoft Edge between supporting CSS transforms on SVG elements and Web Animations - I would say that it's absolutely reasonable to expect they'll land no later than Web Animations - earlier or in same release. And just in case - IE won't get either of those, so shouldn't be affected.

Hope this helps!

dvoytenko · 2017-07-13T00:36:26Z

@boggydigital Thanks a lot! This is super helpful!

/cc @rachelnabors who I was also able to catch recently with on the subject. She might have some insight on this later as well.

rachelnabors · 2017-07-13T00:44:28Z

Haha, @boggydigital is the metatron when it comes to SVG on Edge, as far as I'm concerned!

alancutter · 2017-07-13T01:03:27Z

Thanks for doing the research!

The maintenance burden is low (given signals that Edge will fix SVG CSS transform before implementing element.animate()) while the impact for users is moderate. This looks good to proceed to me. (:

dvoytenko · 2017-07-13T17:45:23Z

@alancutter Thanks a lot! I've completed tests. PTAL.

alancutter · 2017-07-13T23:30:12Z

src/apply-preserving-inline-style.js

+   * Unfortunately, there's no easy way to feature-detect it.
+   */
+  function updateSvgTransformAttr(window) {
+    if (window._webAnimationsUpdateSvgTransformAttr == null) {


Use in instead of weak ==.

Do you not use obfuscation of private vars?

It appears that we don't. We don't obfuscate the tests so this wouldn't work anyway if we did.

alancutter · 2017-07-13T23:31:08Z

src/apply-preserving-inline-style.js

    // Stores the inline style of the element on its behalf while the
    // polyfill uses the element's inline style to simulate web animations.
    // This is needed to fake regular inline style CSSOM access on the element.
    this._surrogateStyle = document.createElementNS('http://www.w3.org/1999/xhtml', 'div').style;
    this._style = element.style;
    this._length = 0;
    this._isAnimatedProperty = {};
+    this._updateSvgTransformAttr =
+        element.namespaceURI &&
+        element.namespaceURI.indexOf('/svg') != -1 &&


Make element a parameter to the helper function instead of having the logic split up like this.

alancutter · 2017-07-13T23:32:26Z

test/js/apply-preserving-inline-style.js

+    this.svgContainer = document.createElementNS(
+        'http://www.w3.org/2000/svg', 'svg');
+    document.documentElement.appendChild(this.svgContainer);
+    window._webAnimationsUpdateSvgTransformAttr = null;


I'd prefer setup/teardown to delete the member rather than set to null.

alancutter · 2017-07-13T23:34:05Z

test/js/apply-preserving-inline-style.js

+        ' AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.20' +
+        ' Mobile Safari/537.36');
+    assert.equal(updateSvgTransformAttr(win), false);
+    // Safary: transforms supported.


alancutter · 2017-07-13T23:34:59Z

test/js/apply-preserving-inline-style.js

@@ -69,4 +75,114 @@ suite('apply-preserving-inline-style', function() {
    this.style.cssText = 'top: 0px';
    assert.equal(this.style.length, 1);
  });
+  test('Detect SVG transform compatibility', function() {
+    var win = {navigator: {userAgent: ''}};


It's a bit strange to have this local variable, agent() can just return an object.

alancutter · 2017-07-13T23:45:28Z

src/property-names.js

@@ -15,12 +15,14 @@
 (function(scope, testing) {

  var aliased = {};
+  var canonical = {};


I find the name "canonical" not very obvious, I have to read the logic to understand what it means. Perhaps renaming them as s/aliased/prefixed/ and s/canonical/unprefixed/ would be clearer.

alancutter · 2017-07-13T23:47:19Z

test/js/apply-preserving-inline-style.js

+    assert.equal(getComputedStyle(svgElement).transform, 'none');
+    assert.equal(svgElement.hasAttribute('transform'), false);
+  });
+  test('Set and clear NOT supported transform on SVG element', function() {


I don't find the phrase "NOT supported transform on SVG element" very clear. Perhaps "transform CSS property not supported on SVG element".

dvoytenko · 2017-07-14T05:15:34Z

@alancutter All changes done. PTAL.

dvoytenko · 2017-07-14T05:56:06Z

@alancutter The test failure does not appear to be related. Could you rerun travis?

battmanz · 2018-08-16T21:38:50Z

@dvoytenko In your original post, you said:

If this change is acceptable, I'll add tests, add more features (e.g. transform-origin) and more docs.

I'm trying to make transform-origin work on an SVG circle element in IE11, but so far all attempts have failed. Is there an open bug for tracking that? Has it already been fixed and I'm just doing it wrong? I've tried it both in CSS as well as JavaScript. Any guidance you can give me is appreciated. Thanks!

dvoytenko · 2018-08-17T20:58:52Z

@battmanz Yeah, transform-origin turned out to be a bit of a can of worms. See:

Turns out most of browsers do not implement transform-origin correctly.

birtles · 2018-08-19T23:58:51Z

Turns out most of browsers do not implement transform-origin correctly.

I think Firefox and Chrome should be right now. Previously there was a problem with Chrome using the wrong value for transform-box but I believe that is fixed that now.

dvoytenko · 2018-08-20T20:32:28Z

@birtles not sure. Related bugs are still open and visibly not a lot of progress.

birtles · 2018-08-20T22:27:02Z

The Firefox bug is closed invalid, and Chrome bug 595829 says that transform-box shipped in M64 so it might be that the Chrome bug 740300 has simply yet to be updated.

dvoytenko · 2018-08-21T23:54:40Z

@birtles Just tried on Chrome 68/69 and the issue is still there with incorrect origin computed values. But for IE it's not really important. Transform-origin can be supported then for IE with some effort, but with a few caveats:

transform-box is not supported on IE, so there's no good way to customize it via CSS. We would either need to build on the default value, or possibly allow re-specifying it as an attribute just for IE purposes. Not a clean approach, but could be a good compromise.
transform-origin CSS is likewise not supported. So we'd likewise need to specify it via an attribute.
We'd need to apply a little math to recalculate the actual transform based on origin and box values.
That'd leave the issues with initial styling, but probably easy enough to leave it out and include it in the "known issues".

Support IE/Edge SVG transforms

267316f

Refine implementation and tests

6eb5bfd

dvoytenko mentioned this pull request Jul 13, 2017

Animatons: support SVG transform animations ampproject/amphtml#10423

Merged

2 tasks

Also include ICS

cd71463

alancutter reviewed Jul 13, 2017

View reviewed changes

review fixes

eda3251

dvoytenko mentioned this pull request Jul 14, 2017

Animations: support SVG transforms ampproject/amphtml#10452

Closed

2 tasks

dvoytenko closed this Jul 17, 2017

dvoytenko reopened this Jul 17, 2017

alancutter merged commit ec226e6 into web-animations:dev Jul 18, 2017

dvoytenko deleted the ie-svg branch July 18, 2017 01:25

mohammed-ibra mentioned this pull request Sep 9, 2024

[Snyk] Upgrade: , webcomponents.js, , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , codemirror, vulcanize, web-animations-js mohammed-ibra/amphtml#444

Open

AKJUS mentioned this pull request Sep 12, 2024

[Snyk] Upgrade: , webcomponents.js, , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , codemirror, vulcanize, web-animations-js AKJUS/amphtml#2

Open

organich mentioned this pull request Sep 13, 2024

[Snyk] Upgrade: , webcomponents.js, , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , codemirror, vulcanize, web-animations-js organich/amphtml#4

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support IE/Edge SVG transforms #148

Support IE/Edge SVG transforms #148

dvoytenko commented Jul 6, 2017 •

edited

Loading

dvoytenko commented Jul 6, 2017

alancutter commented Jul 7, 2017

dvoytenko commented Jul 7, 2017

RByers commented Jul 11, 2017

dvoytenko commented Jul 11, 2017 •

edited

Loading

boggydigital commented Jul 13, 2017

dvoytenko commented Jul 13, 2017

rachelnabors commented Jul 13, 2017

alancutter commented Jul 13, 2017

dvoytenko commented Jul 13, 2017

alancutter Jul 13, 2017

dvoytenko Jul 14, 2017

alancutter Jul 14, 2017

dvoytenko Jul 14, 2017

alancutter Jul 13, 2017

dvoytenko Jul 14, 2017

alancutter Jul 13, 2017

dvoytenko Jul 14, 2017

alancutter Jul 13, 2017

dvoytenko Jul 14, 2017

alancutter Jul 13, 2017

dvoytenko Jul 14, 2017

alancutter Jul 13, 2017

dvoytenko Jul 14, 2017

alancutter Jul 13, 2017

dvoytenko Jul 14, 2017

dvoytenko commented Jul 14, 2017

dvoytenko commented Jul 14, 2017

battmanz commented Aug 16, 2018

dvoytenko commented Aug 17, 2018

birtles commented Aug 19, 2018

dvoytenko commented Aug 20, 2018

birtles commented Aug 20, 2018

dvoytenko commented Aug 21, 2018

Support IE/Edge SVG transforms #148

Support IE/Edge SVG transforms #148

Conversation

dvoytenko commented Jul 6, 2017 • edited Loading

dvoytenko commented Jul 6, 2017

alancutter commented Jul 7, 2017

dvoytenko commented Jul 7, 2017

RByers commented Jul 11, 2017

dvoytenko commented Jul 11, 2017 • edited Loading

boggydigital commented Jul 13, 2017

dvoytenko commented Jul 13, 2017

rachelnabors commented Jul 13, 2017

alancutter commented Jul 13, 2017

dvoytenko commented Jul 13, 2017

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

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

dvoytenko commented Jul 14, 2017

dvoytenko commented Jul 14, 2017

battmanz commented Aug 16, 2018

dvoytenko commented Aug 17, 2018

birtles commented Aug 19, 2018

dvoytenko commented Aug 20, 2018

birtles commented Aug 20, 2018

dvoytenko commented Aug 21, 2018

dvoytenko commented Jul 6, 2017 •

edited

Loading

dvoytenko commented Jul 11, 2017 •

edited

Loading