[Platform-provided behaviors] Prevent subclassing of HTMLSubmitButtonBehavior [chromium/src : main]
Ana Sollano Kim (Gerrit)
Ana Sollano Kim voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Dan Clark
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dan Clark (Gerrit)
Dan Clark added 2 comments![]( "Open in Gerrit")
caseq@ could you please review for `blink/renderer/bindings/OWNERS`?
You may also have an opinion on the question I had of whether this would be better as an interface-level or method-level extended attribute.
[MeasureAs=HTMLSubmitButtonBehaviorUsage, NotSubclassable] constructor();I wonder if `[NotSubclassable]` should actually apply to the interface, not to the constructor itself. Conceptually that seems to be a better fit; we're describing something about the interface as a whole, not the constructor specifically. I think this is clearer when considering the case where a class has multiple constructor overloads; the way we're doing it here, `[NotSubclassable]` would have to be applied to each constructor, and it would be a bug if any were missed.
It's true that there's a comparison to `[HTMLConstructor]`, which does live on the constructor. But that's a little different since `[HTMLConstructor]` replaces the entire constructor with a [different algorithm](https://html.spec.whatwg.org/#htmlconstructor), whereas what we're doing here is just a check that happens to run before the normal constructor steps.
Related details
- Ana Sollano Kim
- Andrey Kosyakov
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ana Sollano Kim (Gerrit)
Ana Sollano Kim added 1 comment![]( "Open in Gerrit")
[MeasureAs=HTMLSubmitButtonBehaviorUsage, NotSubclassable] constructor();I wonder if `[NotSubclassable]` should actually apply to the interface, not to the constructor itself. Conceptually that seems to be a better fit; we're describing something about the interface as a whole, not the constructor specifically. I think this is clearer when considering the case where a class has multiple constructor overloads; the way we're doing it here, `[NotSubclassable]` would have to be applied to each constructor, and it would be a bug if any were missed.
It's true that there's a comparison to `[HTMLConstructor]`, which does live on the constructor. But that's a little different since `[HTMLConstructor]` replaces the entire constructor with a [different algorithm](https://html.spec.whatwg.org/#htmlconstructor), whereas what we're doing here is just a check that happens to run before the normal constructor steps.
I think this makes sense. I updated it to apply to the interface (same in the spec PR). Thanks for the suggestion.
Related details
- Andrey Kosyakov
- Dan Clark
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dan Clark (Gerrit)
Dan Clark voted and added 2 comments![]( "Open in Gerrit")
Votes added by Dan Clark
| Code-Review | +1 |
2 comments
LGTM but I'd still like caseq@ to review for `renderer/bindings`.
E("NotSubclassable", applicable_to=[T.INTERFACE]),Nit: looks like these are alphabetized, so this should go after `NotEnumerable`.
Related details
- Ana Sollano Kim
- Andrey Kosyakov
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrey Kosyakov (Gerrit)
Andrey Kosyakov added 2 comments![]( "Open in Gerrit")
I'm a bit puzzled on what problem this is aiming to solve, considering we haven't done this historically for any other platform objects, can you elaborate on it a bit? What if a user creates an instance of HTMLSubmitButtonBehavior and then manually puts it on a prototype chain of a different object? Are there any security implications?
Also, I assume if PlatformProvidedBehavior becomes a standard, we would also need to standardize the extended attrbute this is introducing, could you file some tracking issue for that in WebIDL spec?
E("NotSubclassable", applicable_to=[T.INTERFACE]),Related details
- Ana Sollano Kim
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |