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

[SecureContext] restrictions on interface vs. interface member are confusing #153

Closed
domenic opened this issue Aug 25, 2016 · 4 comments
Closed
Labels
☕☕ difficulty:medium Hard to fix ⌛⌛ duration:medium Shouldn't be too long to fix

Comments

@domenic
Copy link
Member

domenic commented Aug 25, 2016

Previously at #121 (comment)

The spec currently says

The [SecureContext] extended attribute MUST NOT be specified on both an interface member and the interface or partial interface definition the interface member is declared on.

It's not clear what "declared on" means especially in the context of partial interfaces. E.g. consider the situation

[SecureContext] interface Foo {};
partial interface Foo {
  [SecureContext] void func();
};
@domenic domenic mentioned this issue Aug 25, 2016
bzbarsky pushed a commit that referenced this issue Aug 25, 2016
* Add namespaces

This adds the concept of namespaces, as discussed in whatwg/console#3 (starting especially around whatwg/console#3 (comment)). They can only contain regular operations.

The ES binding for namespaces here is written in a fully modern style, and so differs slightly from similar prose for interfaces' ES binding. It is hoped that it can provide a template for eventually updating interfaces' ES binding to modern ES.

* Address most code review comments:

- Missing some [SecureContext] stuff
- Not yet addressed the operation-creation stuff

* Stick with "declared on" instead of "declared within"

It doesn't read quite right to me, but let's keep it as-is for now and investigate any issues in #153.

* Move "creating an operation function" out to #es-operations
@tobie tobie added ⌛⌛ duration:medium Shouldn't be too long to fix ☕☕ difficulty:medium Hard to fix labels Sep 5, 2016
@domenic
Copy link
Member Author

domenic commented Oct 17, 2017

@tobie this may be fixed with your recent overhaul; can you check?

@tobie
Copy link
Collaborator

tobie commented Oct 17, 2017

So the question is whether we should normatively disallow tautological constructs, do so informatively, or not at all.

If you consider the hierarchy for interfaces or namespaces:

  1. interface/namespace
  2. (partial interface/namespace)
  3. interface/namespace member

and that for interface mixins:

  1. interface
  2. mixin
  3. (partial mixin)
  4. mixin member

Annotating any constructs has a cascading effects on those following (within the same list).

So all of the following are redundant. But not all of them are formally disallowed.

// Your above example;
// tautological but not disallowed.
[SecureContext]
interface Foo {};
partial interface Foo {
  [SecureContext] void func();
};
// Disallowed.
[SecureContext]
interface Foo {
  [SecureContext] void func();
};
// Disallowed.
[SecureContext]
partial interface Foo {
  [SecureContext] void func();
};
// Disallowed.
[SecureContext]
interface mixin Foo {
  [SecureContext] void func();
};
// Disallowed.
[SecureContext]
interface mixin Foo {
  [SecureContext] void func();
};
// tautological but not disallowed. (Same as your example.)
[SecureContext]
interface mixin Foo { }
partial interface mixin Foo { }
  [SecureContext] void func();
};

Then there's the special case of mixins (which can be included in both annotated and non-annotated interfaces and must be annotated in both.

// Allowed
[SecureContext]
interface Foo { }
Foo includes Mixin;

interface Bar { }
Bar includes Mixin;

[SecureContext] 
interface mixin Mixin {
  void func();
};

Either way, we must be consistent and fix this open issue.

@tobie
Copy link
Collaborator

tobie commented Oct 20, 2017

Discussed this offline. Agreed to keep the rules normative but simplify them wherever possible and make them consistent.

Additionally, these are rules that seem worth marking up in a way that Bikeshed and ReSpec can Warn their users about.

@Ms2ger
Copy link
Member

Ms2ger commented Aug 22, 2019

Improved somewhat in #763.

@Ms2ger Ms2ger closed this as completed Aug 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☕☕ difficulty:medium Hard to fix ⌛⌛ duration:medium Shouldn't be too long to fix
Development

No branches or pull requests

3 participants