[Webkit-unassigned] [Bug 20247] setAttributeNode() does not work when attribute name has a capital letter in it

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Aug 22 09:49:11 PDT 2008


https://bugs.webkit.org/show_bug.cgi?id=20247


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #22938|review?                     |review-
               Flag|                            |




------- Comment #7 from darin at apple.com  2008-08-22 09:49 PDT -------
(From update of attachment 22938)
Looks like a great fix!

 10         Add a boolean parameter to getAttributeItem to choose between case
sensitive and case insisitive

Typo here. It should be "insensitive".

 177             if (name == (shouldIgnoreAttributeCase ?
m_attributes[i]->name().toString().lower() :
m_attributes[i]->name().toString()))

Calling lower() is a slower way to do a case insensitive comparison that often
allocates memory. A faster way is to call equalIgnoringCase.

 71     Attribute* getAttributeItem(const String& name, bool
shouldIgnoreAttributeCase = false) const;

Can we do without the default value? How many callers are there that rely on
the default?

What's the performance impact of this change? The function seems simple enough
that I could imagine having multiple copies of it if there are callers who have
no reason to pass the boolean, but on the other hand it's possibly slow enough
already that this extra work has little impact.

 82 function testAttribNodeNamePreservesCaseGetNode2() {

I'd prefer that even JavaScript follow our coding guidelines and put braces on
a separate line for function definitions.

 92     if (!a) {
 93         return "FAIL";
 94     }

And omit braces for one line if statements.

review- because of the equalIgnoringCase issue; please consider the other
comments too.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.



More information about the webkit-unassigned mailing list