[webkit-reviews] review denied: [Bug 15470] Make attr selectors case-sensitive for case-sensitive HTML attrs : [Attachment 16636] Fix (test updates forthcoming)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 15 09:16:23 PDT 2007


Darin Adler <darin at apple.com> has denied Eric Seidel <eric at webkit.org>'s
request for review:
Bug 15470: Make attr selectors case-sensitive for case-sensitive HTML attrs
http://bugs.webkit.org/show_bug.cgi?id=15470

Attachment 16636: Fix (test updates forthcoming)
http://bugs.webkit.org/attachment.cgi?id=16636&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
htmlAttributeHasCaseSensitiveValue should probably use a set instead of 18
separate calls. Can first check that there's no namespace, then use a set of
AtomicString for the attr names.

+	     if (caseSensitive && sel->m_value != value)
+		 return false;
+	     else if (!caseSensitive && !equalIgnoringCase(sel->m_value,
value))
		 return false;

No need to else after return.

I'd write it with ? : instead:

    if (!(caseSensitive ? sel->m_value == value :
equalIgnoringCase(sel->m_value, value)))
	return false;

I think it's a bit unfortunate that we're going to be computing the
caseSensitive boolean all the time. Does every case in the switch statement use
it?

review- for the set issue


More information about the webkit-reviews mailing list