Closed Bug 922160 Opened 6 years ago Closed 6 years ago

Bring EventHandler.webidl more in line with the spec

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: peterv, Assigned: peterv)

Details

Attachments

(2 files, 1 obsolete file)

No description provided.
Comment on attachment 812085 [details] [diff] [review]
Rename BeforeUnloadEventHandler to OnBeforeUnloadEventHandler v1

r=me
Attachment #812085 - Flags: review?(bzbarsky) → review+
Attached patch Remove NodeEventHanders v1 (obsolete) — Splinter Review
This brings EventHandler.webidl in line with the spec. Note that this has the onerror for both nodes and windows as OnErrorEventHandler, just like in the spec. If we don't want that then we need to reintroduce something like NodeEventHandlers.
Attachment #812111 - Flags: review?(bzbarsky)
Doesn't this change the behavior of onerror handlers on non-Window objects?  I believe the spec special-cases Window here somehow at event-handling time, whereas we used to special-case at handler-setting time... and after this patch we don't seem to special-case in either one, so will call onerror handlers on nodes with the wrong arguments?
Flags: needinfo?(peterv)
Which, by the way, is why I'm not convinced that what the spec is doing here is all that sensical.  :(  But I'm tired of arguing with hixie over it.
This leaves onerror mostly alone, because I don't mix that change into the Window api stuff.
Attachment #812111 - Attachment is obsolete: true
Attachment #812111 - Flags: review?(bzbarsky)
Attachment #812235 - Flags: review?(bzbarsky)
Flags: needinfo?(peterv)
Comment on attachment 812235 [details] [diff] [review]
Remove NodeEventHanders v2

>+using nsINode::GetOnmouseenter;                                               \

Why was this not needed before but is needed now?

>+//callback OnBeforeUnloadEventHandlerNonNull = DOMString (Event event);

Maybe point to the spec bug here about the return value needing to be nullable?  Or file one if there isn't one?  Bu I think there is one...

>+// The spec has |attribute OnErrorEventHandler onerror;| on both

s/both //

>+Document implements OnErrorEventHandlerForNodes;

This needs to be in Document.webidl due to the way dependency tracking works.  Similar for the other 3 implements statements here.

>+//Window implements OnErrorEventHandlerForWindow;

And this needs to be in Window.webidl.

r=me with those dealt with.
Attachment #812235 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/2c1f60140a95
https://hg.mozilla.org/mozilla-central/rev/ab00f3093e18
Status: ASSIGNED → RESOLVED
Closed: 6 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.