100188 - Dropdown saves wrong index when using mouse & keyboard

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect)

Description

[jhp (no longer active)]

Build 2001091703.

1) Go to http://www.pc.ibm.com/
2) Click on the dropdown "Please choose a country:" in order to display its
contents.
3) Move your mouse over to "Argentina" so that it is highlighted, but don't click.
4) Press tab to close dropdown and move to the "Go" button.  Notice that the
value selected still shows up as "Please select a country:"
5) Click "Go"

Expected results:
The same page should load since no selection was made

Actual results:
Takes you to Latin-America site.  If you go back to the previous page,
"Argentina" is now selected in the dropdown.

Comment 1

[jhp (no longer active)]

This is causes by different indices stored in nsCombobox vs nsListControlFrame.
 While nsCombobox retains the correct value, nsListControlFrame updates the
index to the element that the mouse highlights.  I was able to fix the problem
with this code in nsCombobox:
  nsComboboxControlFrame::SetFocus(PRBool aOn, PRBool aRepaint)
  {
    if (aOn) {
      mFocused = this;
    } else {
      mFocused = nsnull;
      if (mDroppedDown) {
 +       ((nsListControlFrame*)mListControlFrame)->SetOptionSelected(
 +                                           mSelectedIndex, PR_TRUE );
        ToggleList(mPresContext);
      }
    }

However, this probably is more of a hack than the correct fix.

Comment 2

[rods (gone)]

->>

Comment 3

[jhp (no longer active)]

Any idea about this bug?  It wasn't fixed with the rewrite of 
nsListControlFrame.

Comment 4

[John Keiser (jkeiser)]

Hmm, this must be because when focus is lost from the combobox we are not
properly resetting the value.  This is bad.  I think I will be able to tackle
it, but not until 110800 lands, which touches the same code heavily.

Comment 5

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Major corporations depend on eapp bugs, and need them to be fixed before they
can recommend Mozilla-based products to their customers. Adding nsbeta1+ keyword
and making sure the bugs get re-evaluated if they are targeted beyond 1.0.

Comment 6

[Bob Clary [:bc] (inactive)]

eapp was incorrectly used to change this to nsbeta1+. Resetting to nsbeta1 to
nominate. This is an important issue and deserves to be nsbeta1+ by the ADT.

Comment 7

[John Keiser (jkeiser)]

Attachment: Patch

This fixes the problem by calling the ComboboxFinish() method of the list box,
which checks the selected index and sets it and fires on change if necessary.

Comment 8

[rods (gone)]

r=rods

Comment 9

[Kevin McCluskey (gone)]

Marking nsbeta1+

Comment 10

[Marc Attinasi]

Comment on attachment 69363 [details] [diff] [review]
Patch

sr=attinasi

Comment 11

[John Keiser (jkeiser)]

checked in (last night)

Comment 12

[Madhur Bhatia]

verified fixed on 
win2000 -- build ID : 2002-03-22-05trunk
mac 9.1 -- cuild Id : 2002-03-13-08trunk