Last Comment Bug 809765 - Stop compiling the beforeunload attribute into an event handler on elements other than <body> and <frameset>
: Stop compiling the beforeunload attribute into an event handler on elements o...
Status: RESOLVED FIXED
: dev-doc-needed
Product: Core
Classification: Components
Component: Event Handling (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla19
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on: 807226
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-07 23:36 PST by Boris Zbarsky [:bz]
Modified: 2012-11-25 23:27 PST (History)
4 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Match the HTML spec in terms of which event handler attributes are compiled on which elements. (12.47 KB, patch)
2012-11-12 00:01 PST, Boris Zbarsky [:bz]
bugs: review+
Details | Diff | Review
Match the HTML spec in terms of which event handler attributes are compiled on which elements. (12.41 KB, patch)
2012-11-13 00:24 PST, Boris Zbarsky [:bz]
bugs: review+
Details | Diff | Review

Description Boris Zbarsky [:bz] 2012-11-07 23:36:52 PST
And in general, align our list of attributes we compile with the spec.
Comment 1 Boris Zbarsky [:bz] 2012-11-12 00:01:51 PST
Created attachment 680545 [details] [diff] [review]
Match the HTML spec in terms of which event handler attributes are compiled on which elements.
Comment 2 Olli Pettay [:smaug] 2012-11-12 05:58:30 PST
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] 2012-11-12 09:59:28 PST
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 [:bz] 2012-11-13 00:23:03 PST
> 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 [:bz] 2012-11-13 00:24:37 PST
Filed bug 811217.
Comment 6 Boris Zbarsky [:bz] 2012-11-13 00:24:54 PST
Created attachment 680972 [details] [diff] [review]
Match the HTML spec in terms of which event handler attributes are compiled on which elements.
Comment 7 Olli Pettay [:smaug] 2012-11-13 02:45:34 PST
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 [:bz] 2012-11-14 10:08:23 PST
> 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] 2012-11-14 18:55:39 PST
https://hg.mozilla.org/mozilla-central/rev/5f0036778492

Note You need to log in before you can comment on or make changes to this bug.