809765 - Stop compiling the beforeunload attribute into an event handler on elements other than <body> and <frameset>

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Boris Zbarsky [:bzbarsky]]

And in general, align our list of attributes we compile with the spec.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Match the HTML spec in terms of which event handler attributes are compiled on which elements.

Comment 2

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

Comment on attachment 680545 [details] [diff] [review]
Match the HTML spec in terms of which event handler attributes are compiled on which elements.

> enum EventNameType {
>   EventNameType_None = 0x0000,
>   EventNameType_HTML = 0x0001,
>   EventNameType_XUL = 0x0002,
>   EventNameType_SVGGraphic = 0x0004, // svg graphic elements
>   EventNameType_SVGSVG = 0x0008, // the svg element
>-  EventNameType_SMIL = 0x0016, // smil elements
>+  EventNameType_SMIL = 0x0010, // smil elements
Argh, my bad. I did review the smil change. (Bug 572270)
Brian, what behavior do we want here?
Should nsSVGAnimationElement::IsEventName be changed to
pass EventNameType_SMIL | EventNameType_SVGGraphic ?




r+ for the rest of the patch.

Comment 3

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

Ok, heycam said SVGAnimationElement should support all the svg onfoos.
So nsSVGAnimationElement::IsEventName needs to be changed.
Though, all the svg elements should support same onfoos, I think.

Boris, could you perhaps leave _SMIL handling as it is and file a followup to fix it.
And use perhaps 0x0100 for EventNameType_HTMLBodyOrFramesetOnly.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> Boris, could you perhaps leave _SMIL handling as it is and file a followup to fix it.

Done.

> And use perhaps 0x0100 for EventNameType_HTMLBodyOrFramesetOnly.

Why?  0x20 seems like it should work fine even with SMIL left as 0x16....

Comment 5

[Boris Zbarsky [:bzbarsky]]

Filed bug 811217.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Match the HTML spec in terms of which event handler attributes are compiled on which elements.

Comment 7

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

Comment on attachment 680972 [details] [diff] [review]
Match the HTML spec in terms of which event handler attributes are compiled on which elements.


> 
> enum EventNameType {
>   EventNameType_None = 0x0000,
>   EventNameType_HTML = 0x0001,
>   EventNameType_XUL = 0x0002,
>   EventNameType_SVGGraphic = 0x0004, // svg graphic elements
>   EventNameType_SVGSVG = 0x0008, // the svg element
>   EventNameType_SMIL = 0x0016, // smil elements
>+  EventNameType_HTMLBodyOrFramesetOnly = 0x20,


Well, at least use same syntax as other values. Same amount leading zeros.

>+  // XXXbz Wouldn't having an nsIContent::IsEventAttributeName work better?
Yes. Want to file yet another followup :)

>+  // That way we would only have to put the flags in one place...
>   if (isHtml) {
>-    return nsContentUtils::IsEventAttributeName(aAttrNameAtom, EventNameType_HTML);
>+    return nsContentUtils::IsEventAttributeName(aAttrNameAtom,
>+                                                EventNameType_HTML |
>+                                                EventNameType_HTMLBodyOrFramesetOnly);
>   }
>   else if (isXul) {
>     return nsContentUtils::IsEventAttributeName(aAttrNameAtom, EventNameType_XUL);
>   }
>   else if (isSvg) {
>     return nsContentUtils::IsEventAttributeName(aAttrNameAtom,
>-                                                EventNameType_SVGGraphic | EventNameType_SVGSVG);
>+                                                EventNameType_SVGGraphic |
>+                                                EventNameType_SVGSVG |
>+                                                EventNameType_SMIL);
So this is a minor change to behavior. But should be ok.
(Serializer is a mess)

Comment 8

[Boris Zbarsky [:bzbarsky]]

> Well, at least use same syntax as other values. Same amount leading zeros.

Done.

> Yes. Want to file yet another followup :)

Bug 811753.

http://hg.mozilla.org/integration/mozilla-inbound/rev/5f0036778492

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5f0036778492