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

How to do aliases of legacy interfaces (HTMLDocument, WebKitCSSMatrix...) #362

Closed
zcorpan opened this issue May 12, 2017 · 27 comments
Closed

Comments

@zcorpan
Copy link
Member

zcorpan commented May 12, 2017

So we have this in HTML:

For historical reasons, Window objects must also have a writable, configurable, non-enumerable property named HTMLDocument whose value is the Document interface object.

https://html.spec.whatwg.org/#htmldocument

The intent here is to expose it in the same way as an actual interface is exposed per WebIDL rules.

This method of aliasing legacy interfaces is also used in the Geometry interfaces spec.

For historical reasons, Window objects must also have writable, configurable, non-enumerable properties named SVGMatrix and WebKitCSSMatrix whose value is the DOMMatrix interface object.

https://drafts.fxtf.org/geometry/#webkitcssmatrix

In implementing this it appears tempting to try to do so via an IDL attribute on Window, but that does not have correct behavior (it will have a getter instead of value, it's enumerable, etc). c.f. https://codereview.chromium.org/2709763004/#msg53

Should we add dedicated IDL syntax for this?

@zcorpan
Copy link
Member Author

zcorpan commented May 12, 2017

cc @foolip

@annevk
Copy link
Member

annevk commented May 12, 2017

See whatwg/url#135 (comment) for other use cases.

@zcorpan
Copy link
Member Author

zcorpan commented May 12, 2017

Strawman suggestion:

[Constructor,
 Exposed=Window,
 LegacyAlias=HTMLDocument]
interface Document : Node {
[Constructor(optional (DOMString or sequence<unrestricted double>) init),
 Exposed=(Window,Worker),
 LegacyAlias=(SVGMatrix,WebKitCSSMatrix)]
interface DOMMatrix : DOMMatrixReadOnly {

...with semantics as defined for those right now. Note that the aliases are only on Window; not in workers.

@annevk
Copy link
Member

annevk commented May 12, 2017

LegacyWindowAlias then? (The HTMLDocument alias might go away I just realized, so not sure that should be considered strongly when making a decision.)

@zcorpan
Copy link
Member Author

zcorpan commented May 12, 2017

WFM

@zcorpan
Copy link
Member Author

zcorpan commented May 16, 2017

So an alternative could be to use NamedConstructor. It means WebKitCSSMatrix === DOMMatrix is false but WebKitCSSMatrix.prototype === DOMMatrix.prototype is true. It also allows using a different signature for the old name; e.g. we could have the string argument only for WebKitCSSMatrix...

I searched on httparchive, nerdydata.com, github.com, and couldn't find any instance of "WebKitCSSMatrix.prototype" (just 1 in nerdydata that was in a JS comment).

Would NamedConstructor work for webkitURL?

An alias is probably cleaner, but thought I'd bring this up as a possibility...

@annevk
Copy link
Member

annevk commented May 16, 2017

I don't know, Chromium is probably in a better position to answer such questions.

@zcorpan
Copy link
Member Author

zcorpan commented May 16, 2017

NamedConstructor would break instanceof checks, like this one:

https://github.com/jarodl/Cartog/blob/261f603dc35f569ee137f83fa8b43b566723dc41/lib/uilayer.js#L766

(But I only found 3 instances of this script on github; nothing in httparchive, nothing in nerdydata.)

@zcorpan
Copy link
Member Author

zcorpan commented May 16, 2017

Seeked Chromium opinion in https://codereview.chromium.org/2709763004/#msg81
cc @cdumez if you have an opinion from WebKit perspective.
cc @bzbarsky if you have an opinion from Gecko perspective.

@foolip
Copy link
Member

foolip commented May 16, 2017

I think that aliases (LegacyAlias=(SVGMatrix,WebKitCSSMatrix)) would be better, because we already have aliases for a number of things in Chromium, and when things are exactly identical there are less things that can go wrong when trying to move from a prefixed API to an unprefixed one.

It's possible that NamedConstructor would work out, but I don't volunteer to do the research. These are the existing aliases in Chromium:

  • WebKitTransitionEvent
  • WebKitAnimationEvent
  • webkitURL
  • WebKitMutationObserver
  • webkitMediaStream
  • webkitRTCPeerConnection

There are also a bunch of webkitSpeech*, but they're not only prefixed and use aliases just to keep "webkit" out of most of the code. (You could tell from toString() on the instances of course.)

@zcorpan
Copy link
Member Author

zcorpan commented May 16, 2017

OK then.

webkitURL exists in both windows and workers in WebKit, but only in window in Chromium. The others in your list also do not exist in workers in Chromium. So I suppose it is OK to limit the aliases to window, right?

@zcorpan
Copy link
Member Author

zcorpan commented May 16, 2017

@annevk
Copy link
Member

annevk commented May 16, 2017

@zcorpan
Copy link
Member Author

zcorpan commented May 16, 2017

I tried to specify this in #364. I'm not too familiar with adding things to WebIDL so I probably missed something; review welcome!

@bzbarsky
Copy link
Collaborator

NamedConstructor would break instanceof checks

Why, exactly? I don't think it would. Instanceof checks compare the proto chain of the LHS to the .prototype of the RHS. So as long as the .prototype values match instanceof will be fine.

@bzbarsky
Copy link
Collaborator

That said, I agree aliases are conceptually cleaner. That said, in implementation terms, I'm not aware of Gecko having any aliases right now, and adding them would be a bit of a pain, so will probably delay us shipping anything that involves aliases a good bit, largely because everyone involved in the bindings layer is currently completely swamped with other things.

@zcorpan
Copy link
Member Author

zcorpan commented May 16, 2017

Oh, ok. Then they wouldn't break.

instance.constructor === WebKitCSSMatrix would break, though that seems unlikely to be a common pattern...

@annevk
Copy link
Member

annevk commented May 16, 2017

So maybe we should hold off until Firefox identifies compatibility pain with not having these aliases?

@fserb
Copy link

fserb commented May 16, 2017

Could we have LegacyWindowAlias and use NamedConstructors as a temporary solution on Gecko until their bindings get updated (given that except for the constructor check they are identical)?

It feels to me like a good compromise between not blocking the implementation and having a cleaner interface in the future.

@cdumez
Copy link

cdumez commented May 16, 2017

I also prefer the LegacyAlias (or LegacyWindowAlias) suggestion.

@annevk
Copy link
Member

annevk commented May 16, 2017

@fserb I think it might be the case that Firefox doesn't have these to begin with. E.g., I know for a fact Fx doesn't do webkitURL.

@zcorpan
Copy link
Member Author

zcorpan commented May 16, 2017

@annevk Gecko has WebKitCSSMatrix (but not as an alias).

@zcorpan
Copy link
Member Author

zcorpan commented May 16, 2017

I've learned now that the bindings generator in Chromium for the aliases @foolip mentioned above seems to generate the correct property descriptor. Demo for webkitURL here:

http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5185

The current IDL in Chromium is an attribute on Window:

    [DeprecateAs=PrefixedWindowURL] attribute URLConstructor webkitURL;

@fserb
Copy link

fserb commented May 16, 2017

@annevk, I thought the problem is that it has WebKitCSSMatrix (as @zcorpan pointed out) and we were just discussing how to alias it to DOMMatrix (in this particular instance of the more generic legacy alias problem), right?

So that the new LegacyWindowAlias would be the Right Way (tm), and we should spec as so, but since it's not available anywhere, NamedConstructors would be a reasonable temporary fallback alternative. But maybe I missed something.

@annevk
Copy link
Member

annevk commented May 17, 2017

@fserb yeah, I didn't realize Fx was doing WebKitCSSMatrix in adhoc fashion already.

@tobie
Copy link
Collaborator

tobie commented May 17, 2017

So that the new LegacyWindowAlias would be the Right Way (tm), and we should spec as so, but since it's not available anywhere, NamedConstructors would be a reasonable temporary fallback alternative. But maybe I missed something.

@bzbarsky are you good with this approach? If so, I'll go ahead and merge @zcorpan's pull request #364.

@bzbarsky
Copy link
Collaborator

Speccing LegacyWindowAlias sounds fine. I just can't commit to Firefox implementing it in any particular timeframe.

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

No branches or pull requests

7 participants