Bug 223827

Summary: Allow IDL `dictionary` to be `mixin` and used with `includes` by other `dictionary`
Product: WebKit Reporter: Devin Rousso <hi>
Component: New BugsAssignee: 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 Flags
Patch sam: review-

Description Devin Rousso 2021-03-26 16:24:03 PDT
split off from Bug 223621
Comment 1 Devin Rousso 2021-03-26 16:24:42 PDT
Created attachment 424416 [details]
Patch
Comment 2 Chris Dumez 2021-03-26 17:19:20 PDT
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 3 Darin Adler 2021-03-26 17:31:43 PDT
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?
Comment 4 Chris Dumez 2021-03-26 17:42:11 PDT
(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 5 Devin Rousso 2021-03-26 17:42:53 PDT
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 :)
Comment 6 Sam Weinig 2021-03-26 19:25:22 PDT
(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.
Comment 7 Radar WebKit Bug Importer 2021-04-02 16:26:28 PDT
<rdar://problem/76170322>