Closed Bug 809765 Opened 12 years ago Closed 12 years ago

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

Categories

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

Product:
Core
Component:
DOM: UI Events & Focus Handling
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla19

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

(Keywords: dev-doc-needed)

Attachments

(1 file, 1 obsolete file)

Match the HTML spec in terms of which event handler attributes are compiled on which elements.
12.41 KB, patch
smaug
: review+
Details | Diff | Splinter Review 
And in general, align our list of attributes we compile with the spec.
Assignee: nobody → bzbarsky
Whiteboard: [need review]
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+
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.
Attachment #680545 - Attachment is obsolete: true
Attachment #680545 - Flags: review?(birtles)
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
Closed: 12 years ago
Resolution: --- → FIXED
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: