286170 - User cannot select an OPTION after javascript deletes or adds some others

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Stephen Crawley]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.2) Gecko/20040803
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.2) Gecko/20040803

When a web page uses Javascript to delete unwanted OPTION elements from a 
SELECT, the user may not be able to select other options from the menu,
depending on their position.

Reproducible: Always

Steps to Reproduce:
1. Obtain demo web page.  (I will try to attach it to the submitted bug ...)
2. Follow instructions on page.
3.

Actual Results:  
The user is unable to select the "Equal to" entry in the first menu.
But if the user selects another entry first, the "Equal to" entry
can then be selected.

Expected Results:  
The user should be able to select "Equal to" every time.

I will try to attach the demo web page to the submitted bug.  If
that fails, please contact me.

Comment 1

[Stephen Crawley]

Attachment: A web page that demonstrates the bug

Comment 2

[Stephen Crawley]

I should add that this bug was identified (and the demo page was written) by
Steve Crow at DSTC.

Comment 3

[Daniel Veditz [:dveditz]]

testcase WFM in Firefox 1.0.1, what am I missing? Could you retest with a more
recent build?

Not an exploit, clearing security-confidential flag.

Comment 4

[Stephen Crawley]

Arghhh.  I did a 'SaveAs' to snarff the demobug.html.  This has saved a copy
of the DOM after the Javascript has done its surgery on it ... and that defeats
the test.  I'll replace the file for you.

Comment 5

[Stephen Crawley]

Attachment: The HTML file that demonstrates the bug

Comment 6

[Boris Zbarsky [:bzbarsky]]

Hmm... What's special about the "Equal To" option?  The text?  The value?  Its
position in the <select>?

Comment 7

[Mats Palmgren (inactive)]

Six options are removed, "Equal To" is the sixth element following the selected
element. I'm guessing the control thinks this element is the selected element.

Comment 8

[Mats Palmgren (inactive)]

Attachment: Testcase #2

Comment 9

[Mats Palmgren (inactive)]

If you select the "unselectable" option and then drop down the menu again it
is actually hightlighted. Also, the DOM 'selectedIndex' is correct too,
so the only thing that does not work is the updating of the text in the
select control.

Comment 10

[Mats Palmgren (inactive)]

Attachment: Testcase #3

Comment 11

[Boris Zbarsky [:bzbarsky]]

Interesting.... In testcase #2, after load the selectedIndex is 0, but if I
click twice on the select (to drop it down and roll it back up), it becomes 1...
 Sounds to me like some miscommunication between content and frame.

Comment 12

[Boris Zbarsky [:bzbarsky]]

So the problem is that the combobox has an mDisplayedIndex member that gets out
of sync with the mSelectedIndex member of the <select> in this case....  We
should be updating that member when the DOM mutates.

Mats, do you have time to take a stab at this by any chance?

Comment 13

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 179088 [details] [diff] [review]
Patch rev. 1

>Index: layout/forms/nsComboboxControlFrame.cpp
>+    // When aIndex == mDisplayedIndex the element will select something else,
>+    // see nsHTMLSelectElement::RemoveOptionsFromList().

That's only true if mDisplayIndex is always the same as the select's
mSelectedIndex (in which case there is really no need for mDisplayIndex..).

But the two are out of sync if someone tabs to a combobox and hits the down
arrow, say, no?  If the DOM is then mutated (off a timer, say) and an option is
removed, what behavior do we want, exactly?  I think we want to preserve the
current user selection as much as possible (so most of this patch is right),
but do something sane in the case when our displayed option is removed.

Comment 15

[Mats Palmgren (inactive)]

Attachment: Testcase #6

Comment 16

[Mats Palmgren (inactive)]

Attachment: Testcase #7

Comment 17

[Mats Palmgren (inactive)]

Attachment: Patch rev. 2

> ... do something sane in the case when our displayed option is removed.

I chose to do what IE6/win32 does - select the first option (Testcase #7).

I had a larger patch at first that addressed the case when the hovered option
was removed in a dropdown (Testcase #6) - it re-highlighted what was under the
mouse after the change, but I chose to be compatible with IE6 here also -
select the first option (which is the current behaviour).
The "IsCombobox()" is a leftover from that version...

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 179325 [details] [diff] [review]
Patch rev. 2

r+sr=bzbarsky.	Thanks for fixing this and for cleaning up the content code
some in the bargain!

Comment 19

[Mats Palmgren (inactive)]

Checked in to trunk at 2005-06-20 16:26 PDT

-> FIXED

Comment 20

[Stephen Donner [:stephend] Not actively reading bugmail]

I tested with testcases 1 through 7, and all are working fine for me using build
2005-07-07-06 of SeaMonkey trunk on Windows XP.

Verified FIXED.

Comment 22

[Boris Zbarsky [:bzbarsky]]

Can we get tests for this checked in?  This is historically very fragile behavior that we should be testing for, imo.