Add [LegacyWindowAlias] extended attribute for WebKitCSSMatrix et al #364

zcorpan · 2017-05-16T13:31:38Z

Fixes #362.

Preview | Diff

Fixes whatwg#362.

tobie · 2017-05-16T21:41:56Z

First of all, thanks for doing the work!

A couple of high-level nits:

Would be great to have an example.
Extended attributes are sorted by alphabetical order, so this should go between [LegacyUnenumerableNamedProperties] and [LenientSetter].
The weird line break you'll see around the spec is the result of the XML -> Bikeshed conversion, not something to emulate. Because we can't agree on what to use, however, feel free to use semantic line-breaks or wrap at 100 chars.

Seems it would be useful to include this in §3.6 Interfaces too, no?

zcorpan · 2017-05-17T07:13:20Z

Thanks! Will fix.

Some more todos:

Ban [LegacyWindowAlias] for interfaces that do not include the primary interface object in its exposure set.
Ban [LegacyWindowAlias] for callback interfaces.
Ban >1 [LegacyWindowAlias] for an interface.

zcorpan · 2017-05-17T08:32:32Z

OK, please take another look.

tobie · 2017-05-17T09:27:48Z

index.bs

@@ -10202,6 +10269,21 @@ the <code>typeof</code> operator will return "function" when applied to an inter
 </div>


+<h4 id="legacy-window-alias">Legacy window alias</h4>


I'd move that whole section up in to the introductory section of §3.6 either as second or third ¶ and turn it into something like this:

If the [{{LegacyWindowAlias}}] extended attribute was specified on an interface, then for each [=identifier=] mentioned in the extended attribute, there must be a corresponding property on the [=primary global interface=], known as the <dfn id="dfn-legacy-window-alias" export>legacy window alias</dfn>. The name of the property is the [=legacy window alias=]'s [=identifier=], and its value is a reference to the [=interface object=] for the [=interface=]. The property has attributes { \[[Writable]]: <emu-val>true</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> }.

Done. Only changed "The property has attributes" to "The property has the attributes" for consistency with the surrounding paragraphs.

Follows whatwg/webidl#364

tobie

Once these latests issues are addressed, this LGTM. Would want to hear back from @bzbarsky and FF strategy before merging, however.

tobie · 2017-05-17T12:49:01Z

index.bs

@@ -8725,6 +8727,71 @@ for the specific requirements that the use of
 entails.


+<h4 id="LegacyWindowAlias" extended-attribute lt="LegacyWindowAlias">[LegacyWindowAlias]</h4>


I think you also need to add a number of restrictions around the identifiers and also amend the NamedConstructor description accordingly:

[LegacyWindowAlias] identifiers can't be the same as that of an interface that has an interface object.

[LegacyWindowAlias] identifiers can't be the same as that of a [NamedConstructor].

[LegacyWindowAlias] identifiers can't be the same as that of a [LegacyWindowAlias] used on another interface.

[LegacyWindowAlias] identifiers must not be one of the reserved identifiers.

tobie · 2017-05-17T12:51:02Z

index.bs

+
+If the [{{LegacyWindowAlias}}] [=extended attribute=] appears on an [=interface=],
+it indicates that the [=primary global interface=] will have a property
+for each identifier mentioned in the extended attribute,


Eventually, link identifier.

tobie · 2017-05-17T13:00:28Z

index.bs

+If the [{{LegacyWindowAlias}}] extended attribute was specified on an interface,
+then for each [=identifier=] mentioned in the extended attribute,
+there must be a corresponding property on the [=primary global interface=],
+known as the <dfn id="dfn-legacy-window-alias" export>legacy window alias</dfn>.


I know I wrote that, but this reads poorly: "known as the legacy window alias". It's not super clear whether we're naming the property or the object it references, which already has another name. Maybe not a useful concept to export?

tobie · 2017-05-17T13:03:30Z

index.bs

+then for each [=identifier=] mentioned in the extended attribute,
+there must be a corresponding property on the [=primary global interface=],
+known as the <dfn id="dfn-legacy-window-alias" export>legacy window alias</dfn>.
+The name of the property is the [=legacy window alias=]'s [=identifier=],


What we did for NamedConstructor is define a "NamedConstructor identifier" in the [NamedConstructor] section above. Maybe you'd want something similar here, especially if we get rid of defining the [=legacy window alias=]?

tobie · 2017-05-17T13:12:48Z

index.bs

@@ -10119,6 +10186,15 @@ and its value is an object called the <dfn id="dfn-interface-object" export>inte
 The property has the attributes { \[[Writable]]: <emu-val>true</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> }.
 The characteristics of an interface object are described in [[#interface-object]].

+If the [{{LegacyWindowAlias}}] extended attribute was specified on an interface,


Right now this doesn't specify clearly whether the LegacyWindowAlias should be exposed if its related interface isn't (e.g. because of SecureContext). NamedConstructor is more explicit here: "for every [{{NamedConstructor}}] extended attribute on an [=exposed=] interface, […]". Maybe copy that text?

Can't copy it directly (since here it's a just single extended attribute), but certainly makes sense to add [=exposed=].

zcorpan · 2017-05-18T10:51:45Z

OK addressed latest round of feedback.

* Turn "LegacyWindowAlias identifiers" dfn from a list to an identifier. * Fix a forgotten "must not". * Reference identifiers according to their extended attribute.

tobie · 2017-05-18T17:04:22Z

@domenic think you could kindly take a quick look at this? I tuned the language a bit and I'm not sure it's to the pull request's advantage. :(

Outside of that, we're good to go per #362, so I'll just merge it once you give it the thumbs up.

zcorpan

LGTM

tobie · 2017-05-18T19:14:52Z

Sounds like @zcorpan's fine with the changes. Merging.

Follows whatwg/webidl#364

Add [LegacyWindowAlias] extended attribute for WebKitCSSMatrix et al

05f32d3

Fixes whatwg#362.

zcorpan mentioned this pull request May 16, 2017

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

Closed

Address review comments

85f1693

tobie reviewed May 17, 2017

View reviewed changes

Move and rephrase per tobie's comment

44cf513

zcorpan added a commit to web-platform-tests/wpt that referenced this pull request May 17, 2017

[idlharness] Support testing of [LegacyWindowAlias] extended attribute

551b9d9

Follows whatwg/webidl#364

zcorpan mentioned this pull request May 17, 2017

[idlharness] Support testing of [LegacyWindowAlias] extended attribute web-platform-tests/wpt#5966

Merged

tobie reviewed May 17, 2017

View reviewed changes

Address review comments

7b0758c

Edit language.

d346eb0

* Turn "LegacyWindowAlias identifiers" dfn from a list to an identifier. * Fix a forgotten "must not". * Reference identifiers according to their extended attribute.

tobie approved these changes May 18, 2017

View reviewed changes

tobie requested a review from domenic May 18, 2017 17:01

zcorpan commented May 18, 2017

View reviewed changes

tobie merged commit d732f98 into whatwg:master May 18, 2017

zcorpan deleted the zcorpan/legacywindowalias branch May 18, 2017 20:04

zcorpan added a commit to web-platform-tests/wpt that referenced this pull request May 18, 2017

[idlharness] Support testing of [LegacyWindowAlias] extended attribute

2ad8e8e

Follows whatwg/webidl#364

tobie mentioned this pull request Jun 23, 2017

Track PR #364 #376

Closed

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add [LegacyWindowAlias] extended attribute for WebKitCSSMatrix et al #364

Add [LegacyWindowAlias] extended attribute for WebKitCSSMatrix et al #364

zcorpan commented May 16, 2017 •

edited by pr-preview bot

Loading

tobie commented May 16, 2017

zcorpan commented May 17, 2017 •

edited

Loading

zcorpan commented May 17, 2017

tobie May 17, 2017

zcorpan May 17, 2017

tobie left a comment

tobie May 17, 2017

tobie May 17, 2017

tobie May 17, 2017

tobie May 17, 2017

tobie May 17, 2017

zcorpan May 18, 2017

zcorpan commented May 18, 2017

tobie commented May 18, 2017

zcorpan left a comment

tobie commented May 18, 2017

		@@ -10202,6 +10269,21 @@ the <code>typeof</code> operator will return "function" when applied to an inter
		</div>


		<h4 id="legacy-window-alias">Legacy window alias</h4>

		@@ -8725,6 +8727,71 @@ for the specific requirements that the use of
		entails.


		<h4 id="LegacyWindowAlias" extended-attribute lt="LegacyWindowAlias">[LegacyWindowAlias]</h4>

Add [LegacyWindowAlias] extended attribute for WebKitCSSMatrix et al #364

Add [LegacyWindowAlias] extended attribute for WebKitCSSMatrix et al #364

Conversation

zcorpan commented May 16, 2017 • edited by pr-preview bot Loading

tobie commented May 16, 2017

zcorpan commented May 17, 2017 • edited Loading

zcorpan commented May 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobie left a 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

zcorpan commented May 18, 2017

tobie commented May 18, 2017

zcorpan left a comment

Choose a reason for hiding this comment

tobie commented May 18, 2017

zcorpan commented May 16, 2017 •

edited by pr-preview bot

Loading

zcorpan commented May 17, 2017 •

edited

Loading