Closed
Bug 264644
Opened 20 years ago
Closed 20 years ago
[FIXr]event handler creation with createAttribute depends on case of attribute name but should not
Categories
(Core :: DOM: Core & HTML, defect, P3)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla1.8alpha5
People
(Reporter: martin.honnen, Assigned: bzbarsky)
Details
Attachments
(2 files, 1 obsolete file)
2.38 KB,
text/html
|
Details | |
1.62 KB,
patch
|
sicking
:
review+
peterv
:
superreview+
|
Details | Diff | Splinter Review |
In a HTML document (text/html) the case of attribute names doesn't matter, whether you have <input onclick="JavaScript code"> or <input onClick="JavaScript code"> or <input ONCLICK="JavaScript code">, in each case there is then an event handler created that is fired when the click event occurs. However when I try to use the DOM Level 2 Core method createAttribute to create such event handler attributes dynamically with script I have found that Mozilla (tested with Mozilla 1.8a5 Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5) Gecko/20041015 and with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.2) Gecko/20040804) only creates an event handler when the letters are all lower case, that is I use createAttribute('onclick'). With other casing there is an attribute node created but when the element is clicked there is no event handler fired so it seems no event handler is created. I will attach a test case that demonstrates the problem, it contains three statically created HTML checkbox controls with onclick event handlers that all fire when the checkbox is clicked. Additionally three checkbox controls are created dynamically with script where createAttribute and setAttributeNode is used to attempt to create onclick event handlers dynamically but when you click the dynamically created checkboxes you will find that only for the first where the attribute is created with createAttribute('onclick') the event handler is fired while for the other two the event handler is not fired. In my view the creation of an event handler using createAttribute/setAttributeNode in a text/html (HTML) document should not depend on the case of the attribute name therefore I am filing this bug. [Note that I am aware of other DOM ways to add event handlers (e.g. addEventListener) but I think the DOM Core methods should work reliably too.]
Reporter | ||
Comment 1•20 years ago
|
||
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).
Assignee | ||
Comment 2•20 years ago
|
||
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?
Assignee | ||
Comment 3•20 years ago
|
||
Assignee | ||
Comment 4•20 years ago
|
||
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.
Attachment #162355 -
Flags: superreview?(peterv)
Attachment #162355 -
Flags: review?(bugmail)
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?
Assignee | ||
Comment 6•20 years ago
|
||
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.
Assignee | ||
Comment 8•20 years ago
|
||
So just convert the atom back to string and call SetAttribute? Or SetAttributeNS if we have a namespace?
Yeah, sounds good to me.
Assignee | ||
Updated•20 years ago
|
Attachment #162355 -
Flags: superreview?(peterv)
Attachment #162355 -
Flags: superreview-
Attachment #162355 -
Flags: review?(bugmail)
Attachment #162355 -
Flags: review-
Assignee | ||
Comment 10•20 years ago
|
||
Attachment #162355 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #162550 -
Flags: superreview?(peterv)
Attachment #162550 -
Flags: review?(bugmail)
Comment 11•20 years ago
|
||
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 | ||
Updated•20 years ago
|
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
Assignee | ||
Comment 12•20 years ago
|
||
Fixed.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•