Bug 65715 – Remove LegacyDefaultOptionalArguments flag from DOM-related files except Document.idl

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Mark Pilgrim (Google)]

As discussed in IRC, we are migrating our IDL files away from the interface-level "LegacyDefaultOptionalArguments" flag and onto argument-level [Optional] or [Optional=CallWithDefaultValue] flags. This patch migrates all remaining IDL files in the dom/ directory *except* Document.idl. It does not change any behavior, i.e. it does not make any arguments required that were previously optional, nor vice-versa.

All existing tests pass.

[Mark Pilgrim (Google)]

Created attachment 102972 [details]
Patch

[Adam Barth]

Comment on attachment 102972 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=102972&action=review

> Source/WebCore/dom/Clipboard.idl:-32
> -        LegacyDefaultOptionalArguments,

is RequiresAllArguments still needed for setData ?

> Source/WebCore/dom/MediaStreamList.idl:31
> -        MediaStream item(in [IsIndex] unsigned long index);
> +        MediaStream item(in [IsIndex,Optional=CallWithDefaultValue] unsigned long index);

I'm slightly unsure about this one.  The MediaStream stuff is very new, so maybe it doesn't need it?  On the other hand, having all these fake vectors be consistent seems beneficial.

> Source/WebCore/dom/MediaStreamTrackList.idl:31
> -        MediaStreamTrack item(in [IsIndex] unsigned long index);
> +        MediaStreamTrack item(in [IsIndex,Optional=CallWithDefaultValue] unsigned long index);

This probably isn't needed.

[Adam Barth]

Comment on attachment 102972 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=102972&action=review

> Source/WebCore/dom/TouchList.idl:34
> -        Touch item(in unsigned long index);
> +        Touch item(in [Optional=CallWithDefaultValue] unsigned long index);

Same thing here.  TOUCH_EVENTS are very new.  I think we should skip these even though it's somewhat inconsistent.

[Mark Pilgrim (Google)]

(In reply to comment #2)
> (From update of attachment 102972 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=102972&action=review
> 
> > Source/WebCore/dom/Clipboard.idl:-32
> > -        LegacyDefaultOptionalArguments,
> 
> is RequiresAllArguments still needed for setData ?

Yes. [RequiresAllArguments] is different from [RequiresAllArguments=Raise].

> 
> > Source/WebCore/dom/MediaStreamList.idl:31
> > -        MediaStream item(in [IsIndex] unsigned long index);
> > +        MediaStream item(in [IsIndex,Optional=CallWithDefaultValue] unsigned long index);
> 
> I'm slightly unsure about this one.  The MediaStream stuff is very new, so maybe it doesn't need it?  On the other hand, having all these fake vectors be consistent seems beneficial.
> 
> > Source/WebCore/dom/MediaStreamTrackList.idl:31
> > -        MediaStreamTrack item(in [IsIndex] unsigned long index);
> > +        MediaStreamTrack item(in [IsIndex,Optional=CallWithDefaultValue] unsigned long index);
> 
> This probably isn't needed.

New patch coming up...

[Adam Barth]

(In reply to comment #4)
> (In reply to comment #2)
> > (From update of attachment 102972 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=102972&action=review
> > 
> > > Source/WebCore/dom/Clipboard.idl:-32
> > > -        LegacyDefaultOptionalArguments,
> > 
> > is RequiresAllArguments still needed for setData ?
> 
> Yes. [RequiresAllArguments] is different from [RequiresAllArguments=Raise].

Because it returns undefined?  It might be good to test other browsers to see if we're really not supposed to Raise in this situation.

[Mark Pilgrim (Google)]

Created attachment 103003 [details]
Patch

[Mark Pilgrim (Google)]

This patch changes the behavior of MediaStreamList and MediaTrackList, but it doesn't change Clipboard.

[WebKit Review Bot]

Comment on attachment 103003 [details]
Patch

Clearing flags on attachment: 103003

Committed r92433: <http://trac.webkit.org/changeset/92433>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.