1023762 - No support for sequence members in event codegen

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: test case

This case is found while I try converting MessageEvent to WebIDL codegen.

===Sample WebIDL===
[Constructor(DOMString type, optional TestEventInit eventInitDict)]
interface TestEvent: Event {
  readonly attribute MessagePort[]? ports;
};

dictionary TestEventInit : EventInit {
  sequence<MessagePort>? ports;
};

===Compile error===
0:08.01   File "/Users/hellfire/workspace/hg/mozilla-central/dom/bindings/Codegen.py", line 1382, in define
 0:08.01     return "" if self.inline else self._define()
 0:08.01   File "/Users/hellfire/workspace/hg/mozilla-central/dom/bindings/Codegen.py", line 1378, in _define
 0:08.01     self.indent_body(self.definition_body()) +
 0:08.01   File "/Users/hellfire/workspace/hg/mozilla-central/dom/bindings/Codegen.py", line 7230, in definition_body
 0:08.01     self.attr)
 0:08.01   File "/Users/hellfire/workspace/hg/mozilla-central/dom/bindings/Codegen.py", line 7275, in makeNativeName
 0:08.01     False)
 0:08.01   File "/Users/hellfire/workspace/hg/mozilla-central/dom/bindings/Codegen.py", line 5717, in getRetvalDeclarationForType
 0:08.01     returnType)
 0:08.01 TypeError: Don't know how to declare return value for MessagePort (Wrapper)ArrayOrNull

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

T[] is not supported, and probably won't be.

Comment 2

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Using |[Cached, Pure] readonly attribute sequence<MessagePort>?| will encounter a different error.

 0:07.97   File "/Users/hellfire/workspace/hg/mozilla-central/dom/bindings/mozwebidlcodegen/__init__.py", line 469, in _generate_build_files_for_webidl
 0:07.97     generated_event = CGEventRoot(self._config, stem)
 0:07.97   File "/Users/hellfire/workspace/hg/mozilla-central/dom/bindings/Codegen.py", line 13810, in __init__
 0:07.97     self.root = CGWrapper(CGEventClass(descriptor),
 0:07.97   File "/Users/hellfire/workspace/hg/mozilla-central/dom/bindings/Codegen.py", line 13630, in __init__
 0:07.97     CGBindingImplClass.__init__(self, descriptor, CGEventMethod, CGEventGetter, CGEventSetter, False)
 0:07.97   File "/Users/hellfire/workspace/hg/mozilla-central/dom/bindings/Codegen.py", line 11879, in __init__
 0:07.97     self.methodDecls.append(cgGetter(descriptor, m))
 0:07.97   File "/Users/hellfire/workspace/hg/mozilla-central/dom/bindings/Codegen.py", line 13477, in __init__
 0:07.97     self.body = self.getMethodBody()
 0:07.97   File "/Users/hellfire/workspace/hg/mozilla-central/dom/bindings/Codegen.py", line 13513, in getMethodBody
 0:07.98     raise TypeError("Event code generator does not support this type!")
 0:07.98 TypeError: Event code generator does not support this type!

Comment 3

[Boris Zbarsky [:bzbarsky]]

> T[] is not supported, and probably won't be.

And will be removed from the WebIDL spec, for that matter.

Sequence is a different story.  One issue with sequence is that there is no sane default value to use.  Maybe we should just go ahead and add something like https://www.w3.org/Bugs/Public/show_bug.cgi?id=25391 and then go ahead and implement this?

schien, you probably want to also be ccing smaug on event codegen issues.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Sane default would be null in this case, since sequence<MessagePort>? ports; has '?'.

Also, event codegen won't probably handle all the possible attribute types, so in case an
Event interface uses some rarely used webidl feature, one just needs to implement it manually.

However, Sequence<Foo>? sounds like a type which should be supported.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Oh, it's a nullable sequence, not a sequence of nullables.  OK, great.  ;)

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Oh, and in case of MessageEvent, there isn't really any strong reason to make it use event codegen atm, since it is implemented manually, not using .idl codegen.

Comment 7

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

(In reply to Olli Pettay [:smaug] from comment #6)
> Oh, and in case of MessageEvent, there isn't really any strong reason to
> make it use event codegen atm, since it is implemented manually, not using
> .idl codegen.

The requirement comes from bug 745283. I try codegen UDPMessageEvent with constructor but failed because MessageEvent is implemented manually and not compliant with our code generator. That's the reason I try converting MessageEvent to WebIDL codegen.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

I see.

I think sequence<Foo>? should be supported, though, I might do it first without support for dictionaries.

Comment 9

[Masatoshi Kimura [:emk]]

When T[] is removed from the spec, how Element.attributes will be?
http://dom.spec.whatwg.org/#interface-element

We don't care about the |event.ports !== event.ports| issue anymore?

Comment 10

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

We do; the plan (AIUI) is that event.ports will return the same JS Array, unless the contents have been changed since the last access.

Comment 11

[Boris Zbarsky [:bzbarsky]]

> how Element.attributes will be?

Either an array or an interface with an indexed getter.  It's not clear which yet, not least because it's not clear what the future of Attr is in general.

> We don't care about the |event.ports !== event.ports| issue anymore?

What ms2ger said.  It'll be cached, cleared on change.  Note that it's not like we use a MessagePort[] for .ports right now either!

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Factor out the IDL-to-C++ type mapping for event codegen into a separate function

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Add basic support for sequences of non-containers to event codegen

Comment 14

[Boris Zbarsky [:bzbarsky]]

Attachment: IDL I used to test this

Comment 15

[Boris Zbarsky [:bzbarsky]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/9239175a41ba
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/b549e08e75bc

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/9239175a41ba
https://hg.mozilla.org/mozilla-central/rev/b549e08e75bc