New issue
Advanced search Search tips

Issue 630986 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 569301

Blocking:
issue 630989



Sign in to add a comment

WebIDL callback interfaces are incorrectly implemented

Project Member Reported by hbos@chromium.org, Jul 25 2016

Issue description

A WebIDL callback interface[1] can either be a callback function:

  function(x) { console.log(x); }

Or an object with a handleEvent function:

  var callback = {
    handleEvent:function(x) { console.log(x); }
  };

But the current implementation only supports the callback function case.

Note that WebIDL also has something called "callback function"[2]. It would seem that in most cases when a callback function is desired, a callback interface has been used instead.
If this bug is resolved, we might want to change all current usages of callback interface to callback function - see  issue 569301  about supporting "callback functions".

This bug was spawned by the discussion in [3].

[1] https://heycam.github.io/webidl/#dfn-callback-interface
[2] https://heycam.github.io/webidl/#dfn-callback-function
[3] https://bugs.chromium.org/p/chromium/issues/detail?id=629068#c13

 

Comment 1 by hbos@chromium.org, Jul 25 2016

Cc: haraken@chromium.org bashi@chromium.org foolip@chromium.org yukishiino@chromium.org

Comment 2 by hbos@chromium.org, Jul 25 2016

Blocking: 630989

Comment 3 by foolip@chromium.org, Jul 25 2016

I think that the EventListener and NodeFilter callback interfaces work, i.e. one can pass { handleEvent: function() {} } and { acceptNode: function() {} }, are these the only two that work?

When fixing this, it would be very important to first convert existing callback interfaces to callback functions where required by the spec, to not open up for objects with handleEvent attributes everywhere, which would be silly.

Comment 4 by bashi@chromium.org, Jul 25 2016

Status: Available (was: Untriaged)
The code generator has many special casing for EventListener and it looks like NodeFilter is an interface in Blink so yes, these two should be working.

Comment 5 by foolip@chromium.org, Jul 26 2016

Maybe a good start here would be to change the code generator to treat callback functions exactly like it currently treats all (?) the callback interfaces except EventListener and NodeFilter. In doing so, maybe special-case the interfaces mentioned in https://bugs.chromium.org/p/chromium/issues/detail?id=569301#c15 that currently involve custom bindings, but I think some of those should be possible to migrate afterwards.

Comment 6 by bashi@chromium.org, Jul 26 2016

Components: -Blink>JavaScript>API Blink>Bindings
Yeah, it sounds a good idea. One problem is that the current design of the code generator requires tricky hacks to generate separate .cpp/.h files for callback functions like union types. It may make sense to do refactoring the code generator first.
Project Member

Comment 7 by sheriffbot@chromium.org, Jul 26 2017

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 8 by bashi@chromium.org, Jul 26 2017

Labels: -Hotlist-Recharge-Cold
Owner: bashi@chromium.org
Status: Assigned (was: Untriaged)
Owner: yukishiino@chromium.org
Status: Fixed (was: Assigned)
This was fixed with the following patch.
https://chromium-review.googlesource.com/c/chromium/src/+/763191

Sign in to add a comment