diff --git a/mozilla/layout/forms/nsComboboxControlFrame.cpp b/mozilla/layout/forms/nsComboboxControlFrame.cpp index 0e48c5e8d6e..31a46d24f38 100644 --- a/mozilla/layout/forms/nsComboboxControlFrame.cpp +++ b/mozilla/layout/forms/nsComboboxControlFrame.cpp @@ -185,36 +185,44 @@ nsComboboxControlFrame::IsSuccessful(nsIFormControlFrame* aSubmitter) return (NS_CONTENT_ATTR_HAS_VALUE == GetName(&name)); } - -// Initialize the text string in the combobox using either the current -// selection in the list box or the first item item in the list box. - -void -nsComboboxControlFrame::InitTextStr(PRBool aUpdate) +// If nothing is selected, and we have options, select item 0 +// This is a UI decision that goes against the HTML 4 spec. +// See bugzilla bug 15841 for justification of this deviation. +nsresult +nsComboboxControlFrame::MakeSureSomethingIsSelected() { nsIFormControlFrame* fcFrame = nsnull; nsIFrame* dropdownFrame = GetDropdownFrame(); - nsresult result = dropdownFrame->QueryInterface(kIFormControlFrameIID, (void**)&fcFrame); - - // Update the selected text string - mListControlFrame->GetSelectedItem(mTextStr); - - if (mTextStr == "") { - // No selection so use the first item in the list box - - if ((NS_OK == result) && (nsnull != fcFrame)) { - // Find out if there are any options in the list to select + nsresult rv = dropdownFrame->QueryInterface(kIFormControlFrameIID, (void**)&fcFrame); + if (NS_SUCCEEDED(rv) && fcFrame) { + // If nothing selected, and there are options, default selection to item 0 + rv = mListControlFrame->GetSelectedIndex(&mSelectedIndex); + if (NS_SUCCEEDED(rv) && (mSelectedIndex < 0)) { + // Find out if there are any options in the list to select PRInt32 length = 0; mListControlFrame->GetNumberOfOptions(&length); if (length > 0) { // Set listbox selection to first item in the list box - fcFrame->SetProperty(nsHTMLAtoms::selectedindex, "0"); + rv = fcFrame->SetProperty(nsHTMLAtoms::selectedindex, "0"); mSelectedIndex = 0; - // Get the listbox selection as a string - mListControlFrame->GetSelectedItem(mTextStr); } + UpdateSelection(PR_FALSE, PR_TRUE, mSelectedIndex); // Needed to reflow when removing last option } + + // Don't NS_RELEASE fcFrame here as it isn't addRef'd in the QI (???) } + return rv; +} + +// Initialize the text string in the combobox using either the current +// selection in the list box or the first item item in the list box. +void +nsComboboxControlFrame::InitTextStr(PRBool aUpdate) +{ + MakeSureSomethingIsSelected(); + + // Update the selected text string + mListControlFrame->GetSelectedItem(mTextStr); // Update the display by setting the value attribute mDisplayContent->SetAttribute(kNameSpaceID_None, nsHTMLAtoms::value, mTextStr, aUpdate); @@ -977,7 +985,6 @@ nsComboboxControlFrame::ToggleList(nsIPresContext* aPresContext) return NS_OK; } - NS_IMETHODIMP nsComboboxControlFrame::UpdateSelection(PRBool aDoDispatchEvent, PRBool aForceUpdate, PRInt32 aNewIndex) { @@ -985,8 +992,8 @@ nsComboboxControlFrame::UpdateSelection(PRBool aDoDispatchEvent, PRBool aForceUp // Check to see if the selection changed if (mSelectedIndex != aNewIndex || aForceUpdate) { mListControlFrame->GetSelectedItem(mTextStr); // Update text box - mListControlFrame->UpdateSelection(aDoDispatchEvent, mContent); - } + mListControlFrame->UpdateSelection(aDoDispatchEvent, aForceUpdate, mContent); + } mSelectedIndex = aNewIndex; } @@ -1063,6 +1070,8 @@ nsComboboxControlFrame::AddOption(PRInt32 aIndex) rv = listFrame->AddOption(aIndex); NS_RELEASE(listFrame); } + // If we added the first option, we might need to select it. + MakeSureSomethingIsSelected(); return rv; } @@ -1078,6 +1087,9 @@ nsComboboxControlFrame::RemoveOption(PRInt32 aIndex) rv = listFrame->RemoveOption(aIndex); NS_RELEASE(listFrame); } + // If we removed the selected option, nothing is selected any more. + // Restore selection to option 0 if there are options left. + MakeSureSomethingIsSelected(); return rv; } diff --git a/mozilla/layout/forms/nsComboboxControlFrame.h b/mozilla/layout/forms/nsComboboxControlFrame.h index ba8aa45cf2d..0b3b83f7dc0 100644 --- a/mozilla/layout/forms/nsComboboxControlFrame.h +++ b/mozilla/layout/forms/nsComboboxControlFrame.h @@ -196,7 +196,7 @@ protected: nsIFrame* GetButtonFrame(nsIPresContext& aPresContext); nsIFrame* GetDropdownFrame(); NS_IMETHOD ToggleList(nsIPresContext* aPresContext); - + nsresult MakeSureSomethingIsSelected(); // Default to option 0 nsFrameList mPopupFrames; // additional named child list nsIPresContext* mPresContext; // XXX: Remove the need to cache the pres context. diff --git a/mozilla/layout/forms/nsListControlFrame.cpp b/mozilla/layout/forms/nsListControlFrame.cpp index cebefc83df8..ea2c32a2878 100644 --- a/mozilla/layout/forms/nsListControlFrame.cpp +++ b/mozilla/layout/forms/nsListControlFrame.cpp @@ -1335,11 +1335,6 @@ nsListControlFrame::Reset() } NS_RELEASE(options); - // Start with index 0 selected if nothing is selected - if (mSelectedIndex == kNothingSelected) { - mSelectedIndex = 0; - SetContentSelected(0, PR_TRUE); - } InitSelectionCache(numOptions); } @@ -1486,9 +1481,7 @@ nsListControlFrame::GetSelectedItem(nsString & aStr) if (numOptions == 0) { rv = NS_OK; } else { - PRInt32 selectedIndex = (mSelectedIndex == kNothingSelected ? 0 : mSelectedIndex); - - nsIDOMHTMLOptionElement* optionElement = GetOption(*options, selectedIndex); + nsIDOMHTMLOptionElement* optionElement = GetOption(*options, mSelectedIndex); if (nsnull != optionElement) { nsAutoString text; rv = optionElement->GetLabel(text); @@ -1605,21 +1598,18 @@ nsListControlFrame::AddOption(PRInt32 aIndex) PRInt32 numOptions; GetNumberOfOptions(&numOptions); - PRInt32 selectedIndex; - GetSelectedIndexFromDOM(&selectedIndex); // comes from the DOM +// PRInt32 oldSelectedIndex = mSelectedIndex; + GetSelectedIndexFromDOM(&mSelectedIndex); // comes from the DOM - if (1 == numOptions || mSelectedIndex == kNothingSelected || selectedIndex != mSelectedIndex) { - mSelectedIndex = selectedIndex; - if (mComboboxFrame) { - mComboboxFrame->UpdateSelection(PR_FALSE, PR_TRUE, selectedIndex); // don't dispatch event - } - } else { - mSelectedIndex = selectedIndex; - InitSelectionCache(-1); // Reset sel cache so as not to send event - } + // Adding an option to the select can cause a change in selection + // if the new option has it's selected attribute set. + // selectionChanged = (1 == numOptions || + // kNothingSelected == oldSelectedIndex || mSelectedIndex != oldSelectedIndex) + + // For now, we don't care because we're not dispatching an event: + InitSelectionCache(-1); // Reset sel cache so as not to send event return NS_OK; } - NS_IMETHODIMP nsListControlFrame::RemoveOption(PRInt32 aIndex) @@ -1627,20 +1617,22 @@ nsListControlFrame::RemoveOption(PRInt32 aIndex) PRInt32 numOptions; GetNumberOfOptions(&numOptions); - PRInt32 selectedIndex; - GetSelectedIndexFromDOM(&selectedIndex); // comes from the DOM +// PRInt32 oldSelectedIndex = mSelectedIndex; + GetSelectedIndexFromDOM(&mSelectedIndex); // comes from the DOM - if (aIndex == mSelectedIndex) { - // Don't need to deselect option as it is being removed anyway. - SetContentSelected(selectedIndex, PR_TRUE); // Select the new selectedIndex - } - mSelectedIndex = selectedIndex; - if (nsnull != mComboboxFrame) { - mComboboxFrame->UpdateSelection(PR_FALSE, PR_TRUE, selectedIndex); // don't dispatch event - } else { - InitSelectionCache(-1); // Reset cache to not send event + // Select the new selectedIndex + // Don't need to deselect option as it is being removed anyway. + if (mSelectedIndex >= 0) { + SetContentSelected(mSelectedIndex, PR_TRUE); } + // Only if aIndex != -1 can we determine if there was a change in selection + // selectionChanged = (aIndex == mSelectedIndex) || (mSelectedIndex == + // oldSelectedIndex - (oldSelectedIndex > aIndex)?1:0); + // Call SelectionChanged to dispatch an event if so! + + // For now, we don't care because we're not dispatching an event: + InitSelectionCache(-1); // Reset cache to not send event return NS_OK; } @@ -1711,7 +1703,7 @@ nsListControlFrame::InitSelectionCache(PRInt32 aLength) // Compare content state with local cache of last known state // If there was a change, call SelectionChanged() NS_IMETHODIMP -nsListControlFrame::UpdateSelection(PRBool aDoDispatchEvent, nsIContent* aContent) +nsListControlFrame::UpdateSelection(PRBool aDoDispatchEvent, PRBool aForceUpdate, nsIContent* aContent) { nsresult rv = NS_OK; PRBool changed = PR_FALSE; @@ -1738,13 +1730,11 @@ nsListControlFrame::UpdateSelection(PRBool aDoDispatchEvent, nsIContent* aConten } } - if (changed) { - if (aDoDispatchEvent) { - rv = SelectionChanged(aContent); // Dispatch event - } - if (mComboboxFrame) { - rv = mComboboxFrame->SelectionChanged(); // Update view - } + if (changed && aDoDispatchEvent) { + rv = SelectionChanged(aContent); // Dispatch event + } + if ((changed || aForceUpdate) && mComboboxFrame) { + rv = mComboboxFrame->SelectionChanged(); // Update view } } return rv; @@ -2150,7 +2140,7 @@ nsListControlFrame::MouseUp(nsIDOMEvent* aMouseEvent) } else { mButtonDown = PR_FALSE; CaptureMouseEvents(PR_FALSE); - UpdateSelection(PR_TRUE, mContent); + UpdateSelection(PR_TRUE, PR_FALSE, mContent); } return NS_OK; @@ -2385,7 +2375,7 @@ nsListControlFrame::KeyDown(nsIDOMEvent* aKeyEvent) if (IsInDropDownMode() == PR_TRUE && mComboboxFrame) { mComboboxFrame->ListWasSelected(mPresContext); } else { - UpdateSelection(PR_TRUE, mContent); + UpdateSelection(PR_TRUE, PR_FALSE, mContent); } } if (code == nsIDOMUIEvent::DOM_VK_ESCAPE) { if (IsInDropDownMode() == PR_TRUE && mComboboxFrame) { diff --git a/mozilla/layout/forms/nsListControlFrame.h b/mozilla/layout/forms/nsListControlFrame.h index 428fc304900..781d8736ff0 100644 --- a/mozilla/layout/forms/nsListControlFrame.h +++ b/mozilla/layout/forms/nsListControlFrame.h @@ -119,7 +119,7 @@ public: NS_IMETHOD SyncViewWithFrame(); NS_IMETHOD AboutToDropDown(); NS_IMETHOD AboutToRollup(); - NS_IMETHOD UpdateSelection(PRBool aDoDispatchEvent, nsIContent* aContent); + NS_IMETHOD UpdateSelection(PRBool aDoDispatchEvent, PRBool aForceUpdate, nsIContent* aContent); // nsISelectControlFrame NS_IMETHOD AddOption(PRInt32 index); diff --git a/mozilla/layout/html/forms/src/nsComboboxControlFrame.cpp b/mozilla/layout/html/forms/src/nsComboboxControlFrame.cpp index 0e48c5e8d6e..31a46d24f38 100644 --- a/mozilla/layout/html/forms/src/nsComboboxControlFrame.cpp +++ b/mozilla/layout/html/forms/src/nsComboboxControlFrame.cpp @@ -185,36 +185,44 @@ nsComboboxControlFrame::IsSuccessful(nsIFormControlFrame* aSubmitter) return (NS_CONTENT_ATTR_HAS_VALUE == GetName(&name)); } - -// Initialize the text string in the combobox using either the current -// selection in the list box or the first item item in the list box. - -void -nsComboboxControlFrame::InitTextStr(PRBool aUpdate) +// If nothing is selected, and we have options, select item 0 +// This is a UI decision that goes against the HTML 4 spec. +// See bugzilla bug 15841 for justification of this deviation. +nsresult +nsComboboxControlFrame::MakeSureSomethingIsSelected() { nsIFormControlFrame* fcFrame = nsnull; nsIFrame* dropdownFrame = GetDropdownFrame(); - nsresult result = dropdownFrame->QueryInterface(kIFormControlFrameIID, (void**)&fcFrame); - - // Update the selected text string - mListControlFrame->GetSelectedItem(mTextStr); - - if (mTextStr == "") { - // No selection so use the first item in the list box - - if ((NS_OK == result) && (nsnull != fcFrame)) { - // Find out if there are any options in the list to select + nsresult rv = dropdownFrame->QueryInterface(kIFormControlFrameIID, (void**)&fcFrame); + if (NS_SUCCEEDED(rv) && fcFrame) { + // If nothing selected, and there are options, default selection to item 0 + rv = mListControlFrame->GetSelectedIndex(&mSelectedIndex); + if (NS_SUCCEEDED(rv) && (mSelectedIndex < 0)) { + // Find out if there are any options in the list to select PRInt32 length = 0; mListControlFrame->GetNumberOfOptions(&length); if (length > 0) { // Set listbox selection to first item in the list box - fcFrame->SetProperty(nsHTMLAtoms::selectedindex, "0"); + rv = fcFrame->SetProperty(nsHTMLAtoms::selectedindex, "0"); mSelectedIndex = 0; - // Get the listbox selection as a string - mListControlFrame->GetSelectedItem(mTextStr); } + UpdateSelection(PR_FALSE, PR_TRUE, mSelectedIndex); // Needed to reflow when removing last option } + + // Don't NS_RELEASE fcFrame here as it isn't addRef'd in the QI (???) } + return rv; +} + +// Initialize the text string in the combobox using either the current +// selection in the list box or the first item item in the list box. +void +nsComboboxControlFrame::InitTextStr(PRBool aUpdate) +{ + MakeSureSomethingIsSelected(); + + // Update the selected text string + mListControlFrame->GetSelectedItem(mTextStr); // Update the display by setting the value attribute mDisplayContent->SetAttribute(kNameSpaceID_None, nsHTMLAtoms::value, mTextStr, aUpdate); @@ -977,7 +985,6 @@ nsComboboxControlFrame::ToggleList(nsIPresContext* aPresContext) return NS_OK; } - NS_IMETHODIMP nsComboboxControlFrame::UpdateSelection(PRBool aDoDispatchEvent, PRBool aForceUpdate, PRInt32 aNewIndex) { @@ -985,8 +992,8 @@ nsComboboxControlFrame::UpdateSelection(PRBool aDoDispatchEvent, PRBool aForceUp // Check to see if the selection changed if (mSelectedIndex != aNewIndex || aForceUpdate) { mListControlFrame->GetSelectedItem(mTextStr); // Update text box - mListControlFrame->UpdateSelection(aDoDispatchEvent, mContent); - } + mListControlFrame->UpdateSelection(aDoDispatchEvent, aForceUpdate, mContent); + } mSelectedIndex = aNewIndex; } @@ -1063,6 +1070,8 @@ nsComboboxControlFrame::AddOption(PRInt32 aIndex) rv = listFrame->AddOption(aIndex); NS_RELEASE(listFrame); } + // If we added the first option, we might need to select it. + MakeSureSomethingIsSelected(); return rv; } @@ -1078,6 +1087,9 @@ nsComboboxControlFrame::RemoveOption(PRInt32 aIndex) rv = listFrame->RemoveOption(aIndex); NS_RELEASE(listFrame); } + // If we removed the selected option, nothing is selected any more. + // Restore selection to option 0 if there are options left. + MakeSureSomethingIsSelected(); return rv; } diff --git a/mozilla/layout/html/forms/src/nsComboboxControlFrame.h b/mozilla/layout/html/forms/src/nsComboboxControlFrame.h index ba8aa45cf2d..0b3b83f7dc0 100644 --- a/mozilla/layout/html/forms/src/nsComboboxControlFrame.h +++ b/mozilla/layout/html/forms/src/nsComboboxControlFrame.h @@ -196,7 +196,7 @@ protected: nsIFrame* GetButtonFrame(nsIPresContext& aPresContext); nsIFrame* GetDropdownFrame(); NS_IMETHOD ToggleList(nsIPresContext* aPresContext); - + nsresult MakeSureSomethingIsSelected(); // Default to option 0 nsFrameList mPopupFrames; // additional named child list nsIPresContext* mPresContext; // XXX: Remove the need to cache the pres context. diff --git a/mozilla/layout/html/forms/src/nsListControlFrame.cpp b/mozilla/layout/html/forms/src/nsListControlFrame.cpp index cebefc83df8..ea2c32a2878 100644 --- a/mozilla/layout/html/forms/src/nsListControlFrame.cpp +++ b/mozilla/layout/html/forms/src/nsListControlFrame.cpp @@ -1335,11 +1335,6 @@ nsListControlFrame::Reset() } NS_RELEASE(options); - // Start with index 0 selected if nothing is selected - if (mSelectedIndex == kNothingSelected) { - mSelectedIndex = 0; - SetContentSelected(0, PR_TRUE); - } InitSelectionCache(numOptions); } @@ -1486,9 +1481,7 @@ nsListControlFrame::GetSelectedItem(nsString & aStr) if (numOptions == 0) { rv = NS_OK; } else { - PRInt32 selectedIndex = (mSelectedIndex == kNothingSelected ? 0 : mSelectedIndex); - - nsIDOMHTMLOptionElement* optionElement = GetOption(*options, selectedIndex); + nsIDOMHTMLOptionElement* optionElement = GetOption(*options, mSelectedIndex); if (nsnull != optionElement) { nsAutoString text; rv = optionElement->GetLabel(text); @@ -1605,21 +1598,18 @@ nsListControlFrame::AddOption(PRInt32 aIndex) PRInt32 numOptions; GetNumberOfOptions(&numOptions); - PRInt32 selectedIndex; - GetSelectedIndexFromDOM(&selectedIndex); // comes from the DOM +// PRInt32 oldSelectedIndex = mSelectedIndex; + GetSelectedIndexFromDOM(&mSelectedIndex); // comes from the DOM - if (1 == numOptions || mSelectedIndex == kNothingSelected || selectedIndex != mSelectedIndex) { - mSelectedIndex = selectedIndex; - if (mComboboxFrame) { - mComboboxFrame->UpdateSelection(PR_FALSE, PR_TRUE, selectedIndex); // don't dispatch event - } - } else { - mSelectedIndex = selectedIndex; - InitSelectionCache(-1); // Reset sel cache so as not to send event - } + // Adding an option to the select can cause a change in selection + // if the new option has it's selected attribute set. + // selectionChanged = (1 == numOptions || + // kNothingSelected == oldSelectedIndex || mSelectedIndex != oldSelectedIndex) + + // For now, we don't care because we're not dispatching an event: + InitSelectionCache(-1); // Reset sel cache so as not to send event return NS_OK; } - NS_IMETHODIMP nsListControlFrame::RemoveOption(PRInt32 aIndex) @@ -1627,20 +1617,22 @@ nsListControlFrame::RemoveOption(PRInt32 aIndex) PRInt32 numOptions; GetNumberOfOptions(&numOptions); - PRInt32 selectedIndex; - GetSelectedIndexFromDOM(&selectedIndex); // comes from the DOM +// PRInt32 oldSelectedIndex = mSelectedIndex; + GetSelectedIndexFromDOM(&mSelectedIndex); // comes from the DOM - if (aIndex == mSelectedIndex) { - // Don't need to deselect option as it is being removed anyway. - SetContentSelected(selectedIndex, PR_TRUE); // Select the new selectedIndex - } - mSelectedIndex = selectedIndex; - if (nsnull != mComboboxFrame) { - mComboboxFrame->UpdateSelection(PR_FALSE, PR_TRUE, selectedIndex); // don't dispatch event - } else { - InitSelectionCache(-1); // Reset cache to not send event + // Select the new selectedIndex + // Don't need to deselect option as it is being removed anyway. + if (mSelectedIndex >= 0) { + SetContentSelected(mSelectedIndex, PR_TRUE); } + // Only if aIndex != -1 can we determine if there was a change in selection + // selectionChanged = (aIndex == mSelectedIndex) || (mSelectedIndex == + // oldSelectedIndex - (oldSelectedIndex > aIndex)?1:0); + // Call SelectionChanged to dispatch an event if so! + + // For now, we don't care because we're not dispatching an event: + InitSelectionCache(-1); // Reset cache to not send event return NS_OK; } @@ -1711,7 +1703,7 @@ nsListControlFrame::InitSelectionCache(PRInt32 aLength) // Compare content state with local cache of last known state // If there was a change, call SelectionChanged() NS_IMETHODIMP -nsListControlFrame::UpdateSelection(PRBool aDoDispatchEvent, nsIContent* aContent) +nsListControlFrame::UpdateSelection(PRBool aDoDispatchEvent, PRBool aForceUpdate, nsIContent* aContent) { nsresult rv = NS_OK; PRBool changed = PR_FALSE; @@ -1738,13 +1730,11 @@ nsListControlFrame::UpdateSelection(PRBool aDoDispatchEvent, nsIContent* aConten } } - if (changed) { - if (aDoDispatchEvent) { - rv = SelectionChanged(aContent); // Dispatch event - } - if (mComboboxFrame) { - rv = mComboboxFrame->SelectionChanged(); // Update view - } + if (changed && aDoDispatchEvent) { + rv = SelectionChanged(aContent); // Dispatch event + } + if ((changed || aForceUpdate) && mComboboxFrame) { + rv = mComboboxFrame->SelectionChanged(); // Update view } } return rv; @@ -2150,7 +2140,7 @@ nsListControlFrame::MouseUp(nsIDOMEvent* aMouseEvent) } else { mButtonDown = PR_FALSE; CaptureMouseEvents(PR_FALSE); - UpdateSelection(PR_TRUE, mContent); + UpdateSelection(PR_TRUE, PR_FALSE, mContent); } return NS_OK; @@ -2385,7 +2375,7 @@ nsListControlFrame::KeyDown(nsIDOMEvent* aKeyEvent) if (IsInDropDownMode() == PR_TRUE && mComboboxFrame) { mComboboxFrame->ListWasSelected(mPresContext); } else { - UpdateSelection(PR_TRUE, mContent); + UpdateSelection(PR_TRUE, PR_FALSE, mContent); } } if (code == nsIDOMUIEvent::DOM_VK_ESCAPE) { if (IsInDropDownMode() == PR_TRUE && mComboboxFrame) { diff --git a/mozilla/layout/html/forms/src/nsListControlFrame.h b/mozilla/layout/html/forms/src/nsListControlFrame.h index 428fc304900..781d8736ff0 100644 --- a/mozilla/layout/html/forms/src/nsListControlFrame.h +++ b/mozilla/layout/html/forms/src/nsListControlFrame.h @@ -119,7 +119,7 @@ public: NS_IMETHOD SyncViewWithFrame(); NS_IMETHOD AboutToDropDown(); NS_IMETHOD AboutToRollup(); - NS_IMETHOD UpdateSelection(PRBool aDoDispatchEvent, nsIContent* aContent); + NS_IMETHOD UpdateSelection(PRBool aDoDispatchEvent, PRBool aForceUpdate, nsIContent* aContent); // nsISelectControlFrame NS_IMETHOD AddOption(PRInt32 index);