Created attachment 162299 [details] test case, see description in bug report The test case works flawlessly with Opera 7.60 preview (meaning with that browser both the event handlers of the statically created as well as the event handlers of the dynamically created checkboxes fire when the checkbox is clicked).
The problem here is that createAttribute atomizes the atom name and the event handler check does an atom compare. Which fails if the case is wrong, of course. Should createAttribute on a tag-soup document downcase the incoming attr name like the HTML content sink does?
Comment on attachment 162355 [details] [diff] [review] Like so I went with doing this instead of doing a IsCaseSensitive() check in nsDocument::CreateAttribute, but either way is OK, I guess. In either case, I decided that making createAttributeNS case-insensitive was uncalled for even for tag-soup.
Comment on attachment 162355 [details] [diff] [review] Like so The other option is letting SetAttributeNode go through SetAttribute rather then directly to SetAttr since that will fix the casing. The only problem I can think of is if you create an attribute in an xml document and move it to an html document. Of course, you shouldn't do that without calling importNode. Though maybe importNode on an attribute-node needs fixing too? Looking at importNode though it doesn't fix casing on elements even, which maybe it shouldn't since you should get the same type of element just with a new ownerDocument, which maybe should apply to attributes too. What do you guys think, how should importNode behave on an attribute?
importNode is just severely broken all around (it doesn't even change the ownerDocument; it just clones).
Agreed, but what do we want it to do? The more I think about it the more I think that a better solution is to let SetAttributeNode call SetAttribute. That way they'll always act the same even for attributes created in different documents. That should follow the path of least surprice to everyone. Though I would not be opposed to checking in this patch as well... Martin: While using document.createAttribute certainly should work I would strongly recommend just using element.setAttribute instead. There are a few bugs in the mozilla implementation (partly because noone uses these functions so they get very little testing) and there's a few ambiguities in the spec. On top of this they are inherintly slow in implementation and clumsy in syntax.
So just convert the atom back to string and call SetAttribute? Or SetAttributeNS if we have a namespace?
Yeah, sounds good to me.
Created attachment 162550 [details] [diff] [review] Patch per Jonas' comments
Attachment #162355 - Attachment is obsolete: true
Comment on attachment 162550 [details] [diff] [review] Patch per Jonas' comments Fine by me, note that it'll be slightly slower since we'll end up re-checking the name of the attribute.
Attachment #162550 - Flags: superreview?(peterv) → superreview+
Attachment #162550 - Flags: review?(bugmail) → review+
Assignee: general → bzbarsky
OS: Windows XP → All
Priority: -- → P3
Hardware: PC → All
Summary: event handler creation with createAttribute depends on case of attribute name but should not → [FIXr]event handler creation with createAttribute depends on case of attribute name but should not
Target Milestone: --- → mozilla1.8alpha5
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.