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

Make SVGAnimatedX data types extend DOMX data types wherever possible #175

Closed
AmeliaBR opened this issue Jun 22, 2016 · 12 comments
Closed

Comments

@AmeliaBR
Copy link
Contributor

E.g., make SVGAnimatedString a subclass of DOMString, that just happens to have (redundant, deprecated) getters named animVal and baseVal and a (redundant, deprecated) setter named baseVal. (We've already removed any special meaning from animVal; it is no longer tied to SMIL animation.)

That way, things like SVGAElement.href and SVGElement.className would magically become compatible with HTMLAElement.href and HTMLElement.className for nearly all scripts that use them.

It works fine for JavaScript implementations (strings are objects, they can have properties, getters & setters), but I'm not sure if we're able to extend something like "DOMString" generically.

@dstorey
Copy link
Member

dstorey commented Apr 3, 2018

@AmeliaBR to solve this for SVG href can't we do:

interface mixin SVGURIReference {
  [SameObject, PutForwards=baseVal] readonly attribute SVGAnimatedString href;
};

https://heycam.github.io/webidl/#PutForwards

Providing that works with the fancy xlink:href/href magic then getting/setting href should be the same as href.baseVal

@dstorey
Copy link
Member

dstorey commented Apr 3, 2018

I overlooked something in the above. With setting href you could do the same as HTML: anchor.href = "foo"; (which puts forward to baseVal) but it wouldn't be the same for getting. You'd still get the SVGAnimatedString object of course. Maybe 50% better is still better though?

@AmeliaBR
Copy link
Contributor Author

AmeliaBR commented Apr 3, 2018

@dstorey Halfway there doesn't seem enough to address the main problem: that developers expect these properties to be equivalent to the HTML ones, and then code breaks when it doesn't.

For strings in particular, we could add a simple string serialization for the get case. SVG 2 doesn't currently define stringifiers for these types.

But it would still not be completely consistent with DOMString in a JavaScript binding, where you would expect the value to be a simple string type, which can use . notation to call String object methods.

And that still doesn't address the other SVGAnimated* types.

@dstorey
Copy link
Member

dstorey commented Apr 11, 2018

I discussed this with @travisleithead and after much thought and whiteboard scribblings he came up with a similar approach to you.

There are a few details to work out but we'd need to get something like [LegacyStringClass] defined in WebIDL (similar to[ LegacyArrayClass]) and use it on SVGAnimatedString (or a new replacement....I proposed SVGAnimatedSillyString). Then we'd need to make sure that setting the internal string representation also updated baseVal (and animVal) so that both getting/setting the string directly and href/className.baseVal both work. I think that was the gist of it.

@AmeliaBR
Copy link
Contributor Author

Would this approach be extensible to SVGAnimatedBoolean/Integer/Number?

(And maybe also *Enumeration, which is currently spec'd as basic an SVGAnimatedShort, but without all the artistry and humour usually associated with an "animated short [film]".)

@dstorey
Copy link
Member

dstorey commented Apr 11, 2018

They may need a different [LegacyFoo] but I've not looked into how they're used. If SVGAnimatedNumber returns a number instead of a string (just checked, it returns a float) then I guess it would. I'm not sure if they're as high a priority to fix though as they're not used on something that has direct equivalent in HTML are they? (it would be nice to fix them if it is easy but href is probably the most important thing to fix)

@AmeliaBR
Copy link
Contributor Author

Agree that SVGAnimatedString, and the <a> element interfaces + className are the priorities.

The other SVGAnimatedX interface with X as a base type are only used a few times each. (See full SVG DOM IDL.) And for all the SVG-specific interface properties, the plan (I think) is that CSS Typed OM will eventually make it possible to completely deprecate & replace them, so yeah, it's probably not worth a lot of logical maneuvers to make them more logical as-is.

@travisleithead
Copy link
Member

Filed a WebIDL issue to help get some expert advice.

@longsonr
Copy link

I'm not getting any traction on DOMPoint/DOMRect etc because to use them in SVG I'd have to extend them (so they can be live). I suspect this kind of change would be equally unpalatable.

@AmeliaBR
Copy link
Contributor Author

Resolution of the WebIDL discussion (whatwg/webidl#547 (comment)):

[PutForwards] and stringifier it is. Thanks folks.

This will take care of the two most common needs: being able to get a string value out of these properties (.className and .href and .target), and being able to set them with a string value. There may still be some scripts/libraries designed for HTML that stumble on assumptions about the values being simple strings, but those scripts would be no worse off than they currently are.

@dstorey Are you able to put together a pull request for adding those two features, as well as the changes agreed to in #315 about switching new properties to simple DOMString, and whatever IDL adjustments would need to be made to fix the double-inheritance confusion (#312) ?

Everyone else: Can we get some commitments from implementers to introduce these changes if we spec them?

@dstorey
Copy link
Member

dstorey commented Apr 17, 2018

#315 should be #409 (we agreed to change target to DOMString due to low usage)

#312 is a HTML change and I'll work with them to do that (it is the same issue as moving the members into a shared mixin that would make the above change obsolete)

I'll do another PR to add putForwards/stringifier. I can also implement in Edge. PutForward generates all the code needed automatically so that is a trivial change. stringifier should be straightforward too. At least on baseVal. If we want to add on SVGAElement (probably in href's mixin) to match HTMLAElement.toString() behaviour of returning the href string, then that will need a custom specified stringifier. I believe you do that in WebIDL by specifying new member stringifier and defining the behaviour in prose.

@AmeliaBR
Copy link
Contributor Author

Closing this issue, in favour of a new issue for the chosen behavior: #546

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

6 participants