Bug 90111 – Cleanup HTMLFormCollection

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Ryosuke Niwa]

As the comment at the beginning of HTMLFormCollection.h this entire class is a giant hack, and indeed the class is filled with questionable code.
Clean up some of that mess first so that we can do more refactoring later.

[Ryosuke Niwa]

Created attachment 149824 [details]
Cleanup

[Ryosuke Niwa]

Comment on attachment 149824 [details]
Cleanup

View in context: https://bugs.webkit.org/attachment.cgi?id=149824&action=review

> Source/WebCore/html/HTMLFormCollection.cpp:-176
> -    m_cache.current = getNamedItem(idAttr, name);
> -    if (m_cache.current)
> -        return m_cache.current;
> -    m_cache.current = getNamedItem(nameAttr, name);
> -    return m_cache.current;

Updating m_cache.current here made no sense. In fact, I bet you can find a test case where this introduces a bug using the Objective-C binding.
e.g. collection.item(0); collection.namedItem('blah'); collection.item(0);
The second call to item() returns the element with id/name 'blah'.
Unfortunately, ES5 binding uses namedItems so we can't add a layout test for it.

[Andreas Kling]

Comment on attachment 149824 [details]
Cleanup

View in context: https://bugs.webkit.org/attachment.cgi?id=149824&action=review

> Source/WebCore/ChangeLog:8
> +        Got rid of getNamedItem and enamed getNamedFormItem to firstNamedItem and got rid of duplicateNumber argument since

Typo, enamed.

> Source/WebCore/html/HTMLFormCollection.cpp:126
> +        if (elementsArray[i]->isEnumeratable() && element->fastGetAttribute(attrName) == name)

Maybe someone should rename isEnumeratable() this to isEnumerable() someday.

[Ryosuke Niwa]

Committed r121478: <http://trac.webkit.org/changeset/121478>

[Ryosuke Niwa]

(In reply to comment #4)
> Committed r121478: <http://trac.webkit.org/changeset/121478>

I've figured out a way to write a WebKit API test so I went ahead and landed it with it. Fixed the typo in http://trac.webkit.org/changeset/121479.