[Webkit-unassigned] [Bug 6282] Adding new Option with new Option(text, value, defaultSelected, selected) fails to update selectedIndex

[bugzilla-daemon at opendarwin.org]

http://bugzilla.opendarwin.org/show_bug.cgi?id=6282


darin at apple.com changed:

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




------- Comment #3 from darin at apple.com  2006-05-07 11:06 PDT -------
(From update of attachment 8143)
Looks good.

+         m_element->setSelectedIndex( u );

We don't use parentheses around arguments like this. It should just be "(u)".

This patch needs a test; we don't land bug fixes without tests. We need a test
that demonstrates that this works, and also that demonstrates that modifying
existing elements does *not* change the selected index (to test the "diff >= 0"
in this patch).

Finally, in the future if not in this patch, I'd like to see a lot more of this
in HTMLSelectElement. We don't want functions like this in the binding itself.
JSHTMLSelectCollection::put should deal only with the conventions of how this
feature works JavaScript-wise. Code like adding dummy option elements to padd
out the array and selecting the newly-created element should be in
HTMLSelectElement.

This function should be something like:

    if (propertyName == "selectedIndex")
        m_element->setSelectedIndex(value->toInt32(exec));
    else if (propertyName == lengthPropertyName)
        m_element->setLength(value->toInt32(exec));
    else {
        bool ok;
        unsigned i = propertyName.toUInt32(&ok);
        if (ok)
            m_element->setOption(i, toNode(value));
    }

It might have to be slightly more complicated than that, but should not be a
lot more. The logic that's really about how select elements behave should be in
HTMLSelectElement rather than the JavaScript binding.

review- because of the lack of a test.


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