Sync HTML element interfaces C-D with the spec

Sync HTML element interfaces C-D with the spec

There are no changes to the generated code except ordering.

BUG=460722
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=191547

[philipj_slow, 2015-03-08 09:28:18]

philipj@opera.com changed reviewers:
+ jl@opera.com

[philipj_slow, 2015-03-08 09:28:18]

PTAL

[philipj_slow, 2015-03-08 09:28:40]

A-B is in https://codereview.chromium.org/991513002/ if you want to take a look.

[Jens Widell, 2015-03-08 09:36:46]

https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLCanvasE...
File Source/core/html/HTMLCanvasElement.idl (right):

https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLCanvasE...
Source/core/html/HTMLCanvasElement.idl:52: // trailing arguments should be
variadic.
The variadic bit is something we might not want to fix. It's not observable
whether the argument is (handled as) variadic, and implementing it as such just
makes the code more complicated.

[vivekg, 2015-03-08 09:46:41]

vivekg@chromium.org changed reviewers:
+ vivekg@chromium.org

[vivekg, 2015-03-08 09:46:42]

lgtm with following comment. But that could be taken up separately on blink-dev
may be.

https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLDivElem...
File Source/core/html/HTMLDivElement.idl (right):

https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLDivElem...
Source/core/html/HTMLDivElement.idl:25: [Reflect] attribute DOMString align;
The inline comments like 'obsolete members' might go out-of-place in subsequent
changes and might become irrelevant soonish in a given context.
How about adding an extended attribute to the IDL like:

[Reflect, Obsolete] attribute DOMString align;

This way each attribute/method is properly accounted for being obsolete and 
if required, this could be further used to warn the web developers about the
deprecation.

Also if there exist an alternate way to achieve the same, we could also consider
using 'UseAlternative=XYZ'.

WDYT?

[vivekg, 2015-03-08 09:48:52]

https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLDivElem...
File Source/core/html/HTMLDivElement.idl (right):

https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLDivElem...
Source/core/html/HTMLDivElement.idl:25: [Reflect] attribute DOMString align;
On 2015/03/08 at 09:46:41, vivekg_ wrote:
> 
> Also if there exist an alternate way to achieve the same, we could also
consider using 'UseAlternative=XYZ'.
> 
We could possible reuse 'ImplementedAs=' here as well instead of inventing a new
one.

[philipj_slow, 2015-03-08 14:28:58]

https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLCanvasE...
File Source/core/html/HTMLCanvasElement.idl (right):

https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLCanvasE...
Source/core/html/HTMLCanvasElement.idl:52: // trailing arguments should be
variadic.
On 2015/03/08 09:36:46, Jens Widell wrote:
> The variadic bit is something we might not want to fix. It's not observable
> whether the argument is (handled as) variadic, and implementing it as such
just
> makes the code more complicated. 

Yes, and I think this is a bit weird in the spec, it actually treats all of the
arguments a group, not just argument 2 and following: "Let file be a
serialisation of the canvas element's bitmap as a file, using the method's
arguments (if any) as the arguments."

I'm not sure what the comment should say: "Note that the arguments argument is
variadic in the spec, but that implementing it as such would only complicate the
implementation"?

https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLDivElem...
File Source/core/html/HTMLDivElement.idl (right):

https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLDivElem...
Source/core/html/HTMLDivElement.idl:25: [Reflect] attribute DOMString align;
On 2015/03/08 09:46:41, vivekg_ wrote:
> The inline comments like 'obsolete members' might go out-of-place in
subsequent
> changes and might become irrelevant soonish in a given context.
> How about adding an extended attribute to the IDL like:
> 
> [Reflect, Obsolete] attribute DOMString align;
> 
> This way each attribute/method is properly accounted for being obsolete and 
> if required, this could be further used to warn the web developers about the
> deprecation.
> 
> Also if there exist an alternate way to achieve the same, we could also
consider
> using 'UseAlternative=XYZ'.
> 
> WDYT?

These are the things listed separately in the HTML spec with a link "also has
obsolete members" from the main interface definition. They're not deprecated or
likely to be removed.

I actually kind of wish the HTML spec didn't bother splitting the interfaces,
since these things aren't actually going to be removed.

Is there some clarifying comment I could add to avoid the impression that these
should be deprecated removed?

[Jens Widell, 2015-03-08 15:56:28]

https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLCanvasE...
File Source/core/html/HTMLCanvasElement.idl (right):

https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLCanvasE...
Source/core/html/HTMLCanvasElement.idl:52: // trailing arguments should be
variadic.
On 2015/03/08 14:28:58, philipj_UTC7 wrote:
> On 2015/03/08 09:36:46, Jens Widell wrote:
> > The variadic bit is something we might not want to fix. It's not observable
> > whether the argument is (handled as) variadic, and implementing it as such
> just
> > makes the code more complicated. 
> 
> Yes, and I think this is a bit weird in the spec, it actually treats all of
the
> arguments a group, not just argument 2 and following: "Let file be a
> serialisation of the canvas element's bitmap as a file, using the method's
> arguments (if any) as the arguments."
> 
> I'm not sure what the comment should say: "Note that the arguments argument is
> variadic in the spec, but that implementing it as such would only complicate
the
> implementation"?

"... since only the first argument is used in our implementation"?

I just don't think we should add a FIXME if we don't think it should be fixed,
since chances are someone will come around and simply fix it.

[philipj_slow, 2015-03-09 10:42:45]

turn FIXME into Note

[philipj_slow, 2015-03-09 10:43:25]

https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLCanvasE...
File Source/core/html/HTMLCanvasElement.idl (right):

https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLCanvasE...
Source/core/html/HTMLCanvasElement.idl:52: // trailing arguments should be
variadic.
On 2015/03/08 15:56:28, Jens Widell wrote:
> On 2015/03/08 14:28:58, philipj_UTC7 wrote:
> > On 2015/03/08 09:36:46, Jens Widell wrote:
> > > The variadic bit is something we might not want to fix. It's not
observable
> > > whether the argument is (handled as) variadic, and implementing it as such
> > just
> > > makes the code more complicated. 
> > 
> > Yes, and I think this is a bit weird in the spec, it actually treats all of
> the
> > arguments a group, not just argument 2 and following: "Let file be a
> > serialisation of the canvas element's bitmap as a file, using the method's
> > arguments (if any) as the arguments."
> > 
> > I'm not sure what the comment should say: "Note that the arguments argument
is
> > variadic in the spec, but that implementing it as such would only complicate
> the
> > implementation"?
> 
> "... since only the first argument is used in our implementation"?
> 
> I just don't think we should add a FIXME if we don't think it should be fixed,
> since chances are someone will come around and simply fix it.

I agree, how's the new comment?

[Jens Widell, 2015-03-09 10:53:16]

LGTM

[philipj_slow, 2015-03-09 11:20:08]

https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLDivElem...
File Source/core/html/HTMLDivElement.idl (right):

https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLDivElem...
Source/core/html/HTMLDivElement.idl:25: [Reflect] attribute DOMString align;
On 2015/03/08 14:28:58, philipj_UTC7 wrote:
> On 2015/03/08 09:46:41, vivekg_ wrote:
> > The inline comments like 'obsolete members' might go out-of-place in
> subsequent
> > changes and might become irrelevant soonish in a given context.
> > How about adding an extended attribute to the IDL like:
> > 
> > [Reflect, Obsolete] attribute DOMString align;
> > 
> > This way each attribute/method is properly accounted for being obsolete and 
> > if required, this could be further used to warn the web developers about the
> > deprecation.
> > 
> > Also if there exist an alternate way to achieve the same, we could also
> consider
> > using 'UseAlternative=XYZ'.
> > 
> > WDYT?
> 
> These are the things listed separately in the HTML spec with a link "also has
> obsolete members" from the main interface definition. They're not deprecated
or
> likely to be removed.
> 
> I actually kind of wish the HTML spec didn't bother splitting the interfaces,
> since these things aren't actually going to be removed.
> 
> Is there some clarifying comment I could add to avoid the impression that
these
> should be deprecated removed?

I'm going to land this CL as-is, but if you have suggestions here just say so
and I'll fix this and all of the other instances landed in other CLs.

[philipj_slow, 2015-03-09 11:20:13]

The CQ bit was checked by philipj@opera.com

[philipj_slow, 2015-03-09 11:20:13]

The patchset sent to the CQ was uploaded after l-g-t-m from vivekg@chromium.org
Link to the patchset: https://codereview.chromium.org/988173002/#ps20001 (title:
"turn FIXME into Note")

[commit-bot: I haz the power, 2015-03-09 11:20:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/988173002/20001

[Jens Widell, 2015-03-09 11:27:25]

On 2015/03/09 11:20:08, philipj_UTC7 wrote:
>
https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLDivElem...
> File Source/core/html/HTMLDivElement.idl (right):
> 
>
https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLDivElem...
> Source/core/html/HTMLDivElement.idl:25: [Reflect] attribute DOMString align;
> On 2015/03/08 14:28:58, philipj_UTC7 wrote:
> > On 2015/03/08 09:46:41, vivekg_ wrote:
> > > The inline comments like 'obsolete members' might go out-of-place in
> > subsequent
> > > changes and might become irrelevant soonish in a given context.
> > > How about adding an extended attribute to the IDL like:
> > > 
> > > [Reflect, Obsolete] attribute DOMString align;
> > > 
> > > This way each attribute/method is properly accounted for being obsolete
and 
> > > if required, this could be further used to warn the web developers about
the
> > > deprecation.
> > > 
> > > Also if there exist an alternate way to achieve the same, we could also
> > consider
> > > using 'UseAlternative=XYZ'.
> > > 
> > > WDYT?
> > 
> > These are the things listed separately in the HTML spec with a link "also
has
> > obsolete members" from the main interface definition. They're not deprecated
> or
> > likely to be removed.
> > 
> > I actually kind of wish the HTML spec didn't bother splitting the
interfaces,
> > since these things aren't actually going to be removed.
> > 
> > Is there some clarifying comment I could add to avoid the impression that
> these
> > should be deprecated removed?
> 
> I'm going to land this CL as-is, but if you have suggestions here just say so
> and I'll fix this and all of the other instances landed in other CLs.

In general, I don't think we should add syntax (e.g. extended attributes) that
don't relate to how the interface becomes implemented in the generated code
(unless those extended attributes happen to be in the specification and is just
not used by our implementation, like [NewObject]/[SameObject]).

Comments seem fine for this purpose. We'll need to keep them up-to-date when
modifying the surround IDL declarations, but this is no different from how we
need to handle comments anywhere else (e.g. in C++ code).

[vivekg, 2015-03-09 12:24:01]

On 2015/03/09 11:27:25, Jens Widell wrote:
> On 2015/03/09 11:20:08, philipj_UTC7 wrote:
> >
>
https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLDivElem...
> > File Source/core/html/HTMLDivElement.idl (right):
> > 
> >
>
https://codereview.chromium.org/988173002/diff/1/Source/core/html/HTMLDivElem...
> > Source/core/html/HTMLDivElement.idl:25: [Reflect] attribute DOMString align;
> > On 2015/03/08 14:28:58, philipj_UTC7 wrote:
> > > On 2015/03/08 09:46:41, vivekg_ wrote:
> > > > The inline comments like 'obsolete members' might go out-of-place in
> > > subsequent
> > > > changes and might become irrelevant soonish in a given context.
> > > > How about adding an extended attribute to the IDL like:
> > > > 
> > > > [Reflect, Obsolete] attribute DOMString align;
> > > > 
> > > > This way each attribute/method is properly accounted for being obsolete
> and 
> > > > if required, this could be further used to warn the web developers about
> the
> > > > deprecation.
> > > > 
> > > > Also if there exist an alternate way to achieve the same, we could also
> > > consider
> > > > using 'UseAlternative=XYZ'.
> > > > 
> > > > WDYT?
> > > 
> > > These are the things listed separately in the HTML spec with a link "also
> has
> > > obsolete members" from the main interface definition. They're not
deprecated
> > or
> > > likely to be removed.
> > > 
> > > I actually kind of wish the HTML spec didn't bother splitting the
> interfaces,
> > > since these things aren't actually going to be removed.
> > > 
> > > Is there some clarifying comment I could add to avoid the impression that
> > these
> > > should be deprecated removed?
> > 
> > I'm going to land this CL as-is, but if you have suggestions here just say
so
> > and I'll fix this and all of the other instances landed in other CLs.
> 
> In general, I don't think we should add syntax (e.g. extended attributes) that
> don't relate to how the interface becomes implemented in the generated code
> (unless those extended attributes happen to be in the specification and is
just
> not used by our implementation, like [NewObject]/[SameObject]).
> 
> Comments seem fine for this purpose. We'll need to keep them up-to-date when
> modifying the surround IDL declarations, but this is no different from how we
> need to handle comments anywhere else (e.g. in C++ code).

Agreed, makes sense. My only intention was to be more verbose with the spec. But
given the complexity,  it makes sense to keep the comments in sync. Thanks.

[commit-bot: I haz the power, 2015-03-09 14:03:11]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=191547