Summary: | Allow IDL `dictionary` to be `mixin` and used with `includes` by other `dictionary` | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Devin Rousso <hi> | ||||
Component: | New Bugs | Assignee: | Devin Rousso <hi> | ||||
Status: | ASSIGNED --- | ||||||
Severity: | Normal | CC: | ashvayka, cdumez, darin, esprehn+autocc, ews-watchlist, heycam, hi, kondapallykalyan, rniwa, sam, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=223621 | ||||||
Attachments: |
|
Description
Devin Rousso
2021-03-26 16:24:03 PDT
Created attachment 424416 [details]
Patch
Comment on attachment 424416 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424416&action=review > Source/WebCore/ChangeLog:8 > + Add support for `dictionary mixin` and `partial dictionary mixin`. Since `dictionary` are Why are we doing this? Is this in the WebIDL specification? If so, please provide a link. I just checked the WebIDL specification and could not find it. If this is not standard IDL, I am not convinced we should support this. Comment on attachment 424416 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424416&action=review > Source/WebCore/ChangeLog:11 > + Note that `dictionary mixin` is not officially supported by Web IDL, but it seemed simpler Chris, Devin explicitly says here that it’s not part of WebIDL. Devin, same question: Why are we doing this? (In reply to Darin Adler from comment #3) > Comment on attachment 424416 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=424416&action=review > > > Source/WebCore/ChangeLog:11 > > + Note that `dictionary mixin` is not officially supported by Web IDL, but it seemed simpler > > Chris, Devin explicitly says here that it’s not part of WebIDL. > > Devin, same question: Why are we doing this? Looks like the idea is to avoid some code duplication between dictionaries in the payment request standard and our Apple Pay dictionaries. Even so, I am non convinced it is worth adding non standard IDL to achieve this. Comment on attachment 424416 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424416&action=review >> Source/WebCore/ChangeLog:11 >> + Note that `dictionary mixin` is not officially supported by Web IDL, but it seemed simpler > > Chris, Devin explicitly says here that it’s not part of WebIDL. > > Devin, same question: Why are we doing this? I would like to use this as part of bug 223621 (to avoid having duplicate code between Apple Pay JS and W3C Payment Request API). I'd be happy to discuss this offline next week if that'd help :) (In reply to Chris Dumez from comment #4) > (In reply to Darin Adler from comment #3) > > Comment on attachment 424416 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=424416&action=review > > > > > Source/WebCore/ChangeLog:11 > > > + Note that `dictionary mixin` is not officially supported by Web IDL, but it seemed simpler > > > > Chris, Devin explicitly says here that it’s not part of WebIDL. > > > > Devin, same question: Why are we doing this? > > Looks like the idea is to avoid some code duplication between dictionaries > in the payment request standard and our Apple Pay dictionaries. Even so, I > am non convinced it is worth adding non standard IDL to achieve this. Yeah, I agree. It might seem nice in the short term, but having non-standard stuff in the generator has not really been worth it long term, especially just to avoid some duplication. |