[Webkit-unassigned] [Bug 14955] Implement getElementsByClassName

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=14955


eric at webkit.org changed:

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




------- Comment #18 from eric at webkit.org  2007-10-02 23:43 PDT -------
(From update of attachment 16497)
I am surprised.  I had figured class names were always case insensitive.  It
does not appear to be the case with your patch.  Does contains() need to
lower() the incoming string when in compatMode?

This is not needed, please remove:
+    m_classList = ClassNameList();

Constructor initializers are generally one on each line, with the comma (or
colon) leading the line (see the style guidelines). This allows easy use of #if
ENABLE(FOO) around certain initializers as well as easy removal/addition with
the smallest possible diff.

Maybe it's the need to return "false" when m_impl == 0, that has caused
isLower() not to be implemented before.  One could argue that isLower should
return true for "" or null.  Not sure it matters.  I'm OK having a  funny
isLower as such.  It's also OK to call string->impl()->toLower() (and NOT wrap
it in String(), the compiler will do that for you.)  I know I encouraged you to
add isLower. Doesn't matter either way, your call.

Need to fix the one small style issue, the extra initialization and answer
regarding case insensitive lookups.  I also think we need a compat-mode test
and some case-sensitivity and white-space sensitivity tests before this can
land.

Thanks again for taking the time to work on this.


-- 
Configure bugmail: http://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.