Closed Bug 916879 Opened 11 years ago Closed 11 years ago

|attribute EventHandler onfoo;| should be infallible

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: emk, Assigned: smaug)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

It's ridiculous we will have to write [SetterThrows] every time we define an event handler attribute.
How about we fix our event code to not throw instead?  There's no good reason an event handler setter should ever throw; certainly per spec it doesn't.
Indeed. It is pretty much impossible to throw from ELM::SetEventHandler.
Assignee: nobody → bugs
Attached patch patchSplinter Review
https://tbpl.mozilla.org/?tree=Try&rev=8a62e5443ac1

No changes to workers, since that code will hopefully die soon.
Attachment #805608 - Flags: review?(VYV03354)
Comment on attachment 805608 [details] [diff] [review]
patch

\o/

::: content/events/src/EventTarget.cpp
@@ -27,31 +27,33 @@ EventTarget::GetEventHandler(nsIAtom* aT
> void
> EventTarget::SetEventHandler(const nsAString& aType,
>                              EventHandlerNonNull* aHandler,
>-                             ErrorResult& rv)
>+                             ErrorResult& aRv)
> {
>+  if (!StringBeginsWith(aType, NS_LITERAL_STRING("on"))) {
>+    aRv.Throw(NS_ERROR_INVALID_ARG);
>+    return;
>+  }

Can anyone call this function without a string starting with "on"?
Attachment #805608 - Flags: review?(VYV03354) → review+
Summary: |attribute EventHandler onfoo;| should imply [SetterThrows] → |attribute EventHandler onfoo;| should be infallible
It is for JS implemented EventTargets and they can pass anything.
nsIDOMEventTarget is a builtinclass. Is WebIDL EventTarget implementable with JS?
You can have JS-implemented WebIDL interfaces that inherit from EventTarget.  The EventTarget impl will be in C++, but the JS code will be able to call ChromeOnly methods on it, of course.
Attached patch up-to-dateSplinter Review
And apparently I had missed FMRadio
https://hg.mozilla.org/mozilla-central/rev/8fa498beb805
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: