Bug 170759 – [WebIDL] Add support for extended attributes on types in WebIDL

• Component: Bindings
• Status: RESOLVED FIXED

[Sam Weinig]

With https://github.com/heycam/webidl/pull/286, WebIDL has updated its grammar and added added support for extended attributes directly associated with types.

[Sam Weinig]

Created attachment 306883 [details]
Patch

[Sam Weinig]

Created attachment 306884 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 306884 [details]
Patch

Rejecting attachment 306884 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 306884, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
    -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 215323 = b1e02dbc03afc79fa6594cd2761a7eafee9f7c7a
r215324 = 64df7a4298e2b28020dad16783319373057cd46d
r215325 = 04c71ab1eee5352b02f54676fe40e28c88526a5b
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.webkit.org/results/3529496

[Sam Weinig]

(In reply to WebKit Commit Bot from comment #3)
> Comment on attachment 306884 [details]
> Patch
> 
> Rejecting attachment 306884 [details] from commit-queue.
> 
> Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch',
> '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03',
> 'land-attachment', '--force-clean', '--non-interactive',
> '--parent-command=commit-queue', 306884, '--port=mac']" exit_code: 2 cwd:
> /Volumes/Data/EWS/WebKit
> 
> Last 500 characters of output:
>     -> origin/master
> Partial-rebuilding
> .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-
> d52691b4dbfc ...
> Currently at 215323 = b1e02dbc03afc79fa6594cd2761a7eafee9f7c7a
> r215324 = 64df7a4298e2b28020dad16783319373057cd46d
> r215325 = 04c71ab1eee5352b02f54676fe40e28c88526a5b
> Done rebuilding
> .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-
> d52691b4dbfc
> First, rewinding head to replay your work on top of it...
> Fast-forwarded master to refs/remotes/origin/master.
> 
> Full output: http://webkit-queues.webkit.org/results/3529496

Not going to lie, that output is a tad confusing. But I'll assume it means it didn't merge cleanly.

[Chris Dumez]

Comment on attachment 306884 [details]
Patch

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

> Source/WebCore/css/MediaList.idl:32
> +    [SetterMayThrowException] attribute [TreatNullAs=EmptyString] DOMString mediaText;

Why? This no longer matches WebIDL syntax or the spec:
https://drafts.csswg.org/cssom/#the-medialist-interface

This should be:
[SetterMayThrowException, TreatNullAs=EmptyString] attribute DOMString mediaText;

I do not like this change at all unless this is somehow a newer WebIDL syntax I do not know about.

[Chris Dumez]

(In reply to Chris Dumez from comment #5)
> Comment on attachment 306884 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=306884&action=review
> 
> > Source/WebCore/css/MediaList.idl:32
> > +    [SetterMayThrowException] attribute [TreatNullAs=EmptyString] DOMString mediaText;
> 
> Why? This no longer matches WebIDL syntax or the spec:
> https://drafts.csswg.org/cssom/#the-medialist-interface
> 
> This should be:
> [SetterMayThrowException, TreatNullAs=EmptyString] attribute DOMString
> mediaText;
> 
> I do not like this change at all unless this is somehow a newer WebIDL
> syntax I do not know about.

I noticed this when trying to align MediaList.idl with the spec. I can no longer copy/paste from the spec which is a huge downside for me. Our WebIDL should be as close to specs as possible.

[Chris Dumez]

(In reply to Chris Dumez from comment #6)
> (In reply to Chris Dumez from comment #5)
> > Comment on attachment 306884 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=306884&action=review
> > 
> > > Source/WebCore/css/MediaList.idl:32
> > > +    [SetterMayThrowException] attribute [TreatNullAs=EmptyString] DOMString mediaText;
> > 
> > Why? This no longer matches WebIDL syntax or the spec:
> > https://drafts.csswg.org/cssom/#the-medialist-interface
> > 
> > This should be:
> > [SetterMayThrowException, TreatNullAs=EmptyString] attribute DOMString
> > mediaText;
> > 
> > I do not like this change at all unless this is somehow a newer WebIDL
> > syntax I do not know about.
> 
> I noticed this when trying to align MediaList.idl with the spec. I can no
> longer copy/paste from the spec which is a huge downside for me. Our WebIDL
> should be as close to specs as possible.

I have just confirmed this is not proper WebIDL syntax. The latest spec is at:
https://heycam.github.io/webidl/#TreatNullAs

Note the syntax in the spec is:
[TreatNullAs=EmptyString] attribute DOMString owner;

NOT
attribute [TreatNullAs=EmptyString] DOMString owner;

Can we please roll back this part of the change?

[Chris Dumez]

(In reply to Chris Dumez from comment #7)
> (In reply to Chris Dumez from comment #6)
> > (In reply to Chris Dumez from comment #5)
> > > Comment on attachment 306884 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=306884&action=review
> > > 
> > > > Source/WebCore/css/MediaList.idl:32
> > > > +    [SetterMayThrowException] attribute [TreatNullAs=EmptyString] DOMString mediaText;
> > > 
> > > Why? This no longer matches WebIDL syntax or the spec:
> > > https://drafts.csswg.org/cssom/#the-medialist-interface
> > > 
> > > This should be:
> > > [SetterMayThrowException, TreatNullAs=EmptyString] attribute DOMString
> > > mediaText;
> > > 
> > > I do not like this change at all unless this is somehow a newer WebIDL
> > > syntax I do not know about.
> > 
> > I noticed this when trying to align MediaList.idl with the spec. I can no
> > longer copy/paste from the spec which is a huge downside for me. Our WebIDL
> > should be as close to specs as possible.
> 
> I have just confirmed this is not proper WebIDL syntax. The latest spec is
> at:
> https://heycam.github.io/webidl/#TreatNullAs
> 
> Note the syntax in the spec is:
> [TreatNullAs=EmptyString] attribute DOMString owner;
> 
> NOT
> attribute [TreatNullAs=EmptyString] DOMString owner;
> 
> Can we please roll back this part of the change?

Ok, I have found https://github.com/heycam/webidl/pull/286 which seems to indicate this is a new Web IDL syntax. Therefore, the example I pointed to must be out of date. I have pointed this out upstream.

Such a big change is unfortunate as most specs are out of date now, but anyway. If this is where Web IDL is going, I no longer oppose to this.

[Chris Dumez]

(In reply to Chris Dumez from comment #8)
> (In reply to Chris Dumez from comment #7)
> > (In reply to Chris Dumez from comment #6)
> > > (In reply to Chris Dumez from comment #5)
> > > > Comment on attachment 306884 [details]
> > > > Patch
> > > > 
> > > > View in context:
> > > > https://bugs.webkit.org/attachment.cgi?id=306884&action=review
> > > > 
> > > > > Source/WebCore/css/MediaList.idl:32
> > > > > +    [SetterMayThrowException] attribute [TreatNullAs=EmptyString] DOMString mediaText;
> > > > 
> > > > Why? This no longer matches WebIDL syntax or the spec:
> > > > https://drafts.csswg.org/cssom/#the-medialist-interface
> > > > 
> > > > This should be:
> > > > [SetterMayThrowException, TreatNullAs=EmptyString] attribute DOMString
> > > > mediaText;
> > > > 
> > > > I do not like this change at all unless this is somehow a newer WebIDL
> > > > syntax I do not know about.
> > > 
> > > I noticed this when trying to align MediaList.idl with the spec. I can no
> > > longer copy/paste from the spec which is a huge downside for me. Our WebIDL
> > > should be as close to specs as possible.
> > 
> > I have just confirmed this is not proper WebIDL syntax. The latest spec is
> > at:
> > https://heycam.github.io/webidl/#TreatNullAs
> > 
> > Note the syntax in the spec is:
> > [TreatNullAs=EmptyString] attribute DOMString owner;
> > 
> > NOT
> > attribute [TreatNullAs=EmptyString] DOMString owner;
> > 
> > Can we please roll back this part of the change?
> 
> Ok, I have found https://github.com/heycam/webidl/pull/286 which seems to
> indicate this is a new Web IDL syntax. Therefore, the example I pointed to
> must be out of date. I have pointed this out upstream.
> 
> Such a big change is unfortunate as most specs are out of date now, but
> anyway. If this is where Web IDL is going, I no longer oppose to this.

I filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=30107 to try and get the CSSOM spec updated. Domenic also agreed the example in the WebIDL spec is outdated and will fix it. Sorry about the noise.