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

RESOLVED FIXED in mozilla19

Status

()

Core
Event Handling
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

({dev-doc-needed})

unspecified
mozilla19
x86
Mac OS X
dev-doc-needed
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

And in general, align our list of attributes we compile with the spec.
Created attachment 680545 [details] [diff] [review]
Match the HTML spec in terms of which event handler attributes are compiled on which elements.
Attachment #680545 - Flags: review?(bugs)
Assignee: nobody → bzbarsky
Whiteboard: [need review]

Comment 2

5 years ago
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.
Attachment #680545 - Flags: review?(bugs)
Attachment #680545 - Flags: review?(birtles)
Attachment #680545 - Flags: review+

Comment 3

5 years ago
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.
> 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....
Filed bug 811217.
Created attachment 680972 [details] [diff] [review]
Match the HTML spec in terms of which event handler attributes are compiled on which elements.
Attachment #680972 - Flags: review?(bugs)
Attachment #680545 - Attachment is obsolete: true
Attachment #680545 - Flags: review?(birtles)

Comment 7

5 years ago
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)
Attachment #680972 - Flags: review?(bugs) → review+
> 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
Flags: in-testsuite+
Whiteboard: [need review]
Target Milestone: --- → mozilla19
https://hg.mozilla.org/mozilla-central/rev/5f0036778492
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.