Closed Bug 870219 Opened 6 years ago Closed 6 years ago

Add a way for JS-implemented WebIDL objects to sanely do event handlers

Categories

(Core :: DOM: Events, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

(Keywords: dev-doc-needed)

Attachments

(2 files)

Without having to reinvent the wheel, that is.
With the upcoming patch, implementing something like this:

  attribute EventHandler onaddstream;

would look like this:

  get onaddstream() {
    return this.__DOM_IMPL__.getEventHandler("addstream");
  },
  set onaddstream(handler) {
    return this.__DOM_IMPL__.setEventHandler("addstream", handler);
  },

which is not as cool as if we autogenerated it in the binding code, but better than nothing...

Of course the other option is we could just special-case event handlers in the binding codegen: for things inheriting from EventTarget, attributes of type EventHandler, named on*, just generate calls into the superclass...  Not sure whether it's worth the bother.
Blocks: 823512
No longer blocks: 858741
Blocks: 850430
Attachment #747293 - Flags: review?(bugs) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/52f646b2055c
Flags: in-testsuite-
Whiteboard: [need review]
Target Milestone: --- → mozilla23
Any reason you called it setEventHandler rather than setEventListener?
Yes they're different things.  An event listener is something that gets invoked when an event fires.  An event handler is a callback that a very special kinds of event listener can call...
addEventListener handles event listeners, onfoo properties are event handlers.
And backed out: https://hg.mozilla.org/integration/mozilla-inbound/rev/10cb7bc4cc58

The problem is that worker code is crashing at startup with:

  ###!!! ASSERTION: Wrong thread!: '!NS_IsMainThread()', file ../../../dom/workers/WorkerPrivate.cpp, line 4257

with the stack showing JS running under the component loader and doing:

mozilla::dom::EventTargetBinding_workers::CreateInterfaceObjects(JSContext*, JS::Handle<JSObject*>, JSObject**)+0x00000063 [/builds/slave/m-in-osx64-d-00000000000000000/build/obj-firefox/dist/FirefoxNightlyDebug.app/Contents/MacOS/XUL +0x01882423]
mozilla::dom::workers::ResolveWorkerClasses(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, unsigned int, JS::MutableHandle<JSObject*>)+0x000001BF [/builds/slave/m-in-osx64-d-00000000000000000/build/obj-firefox/dist/FirefoxNightlyDebug.app/Contents/MacOS/XUL +0x00D1679F]
BackstagePass::NewResolve(nsIXPConnectWrappedNative*, JSContext*, JSObject*, jsid, unsigned int, JSObject**, bool*)+0x00000086 [/builds/slave/m-in-osx64-d-00000000000000000/build/obj-firefox/dist/FirefoxNightlyDebug.app/Contents/MacOS/XUL +0x010FAC86]
Comment on attachment 747539 [details] [diff] [review]
part 1.  Fix the chrome-scope checking for workers to work even if we incorrectly set up worker interface objects on the main thread.

Are we guaranteed that nsThreadUtils.h is included somewhere?
Attachment #747539 - Flags: review?(bent.mozilla) → review+
> Are we guaranteed that nsThreadUtils.h is included somewhere?

I'll add it to the list of binding includes, just in case.
You need to log in before you can comment on or make changes to this bug.