[webkit-reviews] review denied: [Bug 120293] setAttributeNode() does not set the new value to an existing attribute if specified attribute is in a different case. : [Attachment 209642] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Arpita Bahuguna
<arpitabahuguna at gmail.com>'s request for review:
Bug 120293: setAttributeNode() does not set the new value to an existing
attribute if specified attribute is in a different case.
https://bugs.webkit.org/show_bug.cgi?id=120293

Attachment 209642: Patch
https://bugs.webkit.org/attachment.cgi?id=209642&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=209642&action=review


Fix looks fine, but need to remove the excess argument for
setAttributeInternal.

>> Source/WebCore/dom/Element.cpp:954
>> +	QualifiedName attrNodeName = attributeAt(index).name();
> 
> This variable should be a const QualifiedName&, to avoid unnecessary
ref()/deref().
> "attrNodeName" is not a good name, as it there isn't necessarily an Attr node
present (yet.)
> I would call this something like "existingAttributeName" or
"nameOfExistingAttribute"

The right name for this is “name”. And the “name” argument to
setAttributeInternal must be removed, because it is not used at all. That’s the
right way to do it. No point in passing an unused argument that would just
introduce a bug if we used it for anything.