From 548bfa10ad792384cdfe73106e5e75bfbf5236a1 Mon Sep 17 00:00:00 2001 From: Mounir Lamouri Date: Fri, 8 Oct 2010 14:15:00 +0200 Subject: [PATCH] Bug 598238 - Don't call GetMultiple but use HasAttr. r=smaug a=sicking --- .../src/html/nsHTMLSelectAccessible.cpp | 37 +++++++------------ .../html/content/src/nsHTMLInputElement.cpp | 4 +- .../html/content/src/nsHTMLSelectElement.cpp | 14 ++----- .../html/content/src/nsHTMLSelectElement.h | 8 ++-- layout/forms/nsListControlFrame.cpp | 21 ----------- layout/forms/nsListControlFrame.h | 8 ++-- 6 files changed, 28 insertions(+), 64 deletions(-) diff --git a/accessible/src/html/nsHTMLSelectAccessible.cpp b/accessible/src/html/nsHTMLSelectAccessible.cpp index 41ebec1b23be..16d586723b1b 100644 --- a/accessible/src/html/nsHTMLSelectAccessible.cpp +++ b/accessible/src/html/nsHTMLSelectAccessible.cpp @@ -80,23 +80,18 @@ nsHTMLSelectListAccessible::GetStateInternal(PRUint32 *aState, // nsIAccessibleStates::STATE_MULTISELECTABLE // nsIAccessibleStates::STATE_EXTSELECTABLE - nsCOMPtr select(do_QueryInterface(mContent)); - if (select) { - if (*aState & nsIAccessibleStates::STATE_FOCUSED) { - // Treat first focusable option node as actual focus, in order - // to avoid confusing JAWS, which needs focus on the option - nsCOMPtr focusedOption = - nsHTMLSelectOptionAccessible::GetFocusedOption(mContent); - if (focusedOption) { // Clear focused state since it is on option - *aState &= ~nsIAccessibleStates::STATE_FOCUSED; - } + if (*aState & nsIAccessibleStates::STATE_FOCUSED) { + // Treat first focusable option node as actual focus, in order + // to avoid confusing JAWS, which needs focus on the option + nsCOMPtr focusedOption = + nsHTMLSelectOptionAccessible::GetFocusedOption(mContent); + if (focusedOption) { // Clear focused state since it is on option + *aState &= ~nsIAccessibleStates::STATE_FOCUSED; } - PRBool multiple; - select->GetMultiple(&multiple); - if ( multiple ) - *aState |= nsIAccessibleStates::STATE_MULTISELECTABLE | - nsIAccessibleStates::STATE_EXTSELECTABLE; } + if (mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::multiple)) + *aState |= nsIAccessibleStates::STATE_MULTISELECTABLE | + nsIAccessibleStates::STATE_EXTSELECTABLE; return NS_OK; } @@ -122,19 +117,15 @@ nsHTMLSelectListAccessible::IsSelect() bool nsHTMLSelectListAccessible::SelectAll() { - nsCOMPtr selectElm(do_QueryInterface(mContent)); - PRBool isMultiple = PR_FALSE; - selectElm->GetMultiple(&isMultiple); - return isMultiple ? nsAccessibleWrap::SelectAll() : false; + return mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::multiple) ? + nsAccessibleWrap::SelectAll() : false; } bool nsHTMLSelectListAccessible::UnselectAll() { - nsCOMPtr selectElm(do_QueryInterface(mContent)); - PRBool isMultiple = PR_FALSE; - selectElm->GetMultiple(&isMultiple); - return isMultiple ? nsAccessibleWrap::UnselectAll() : false; + return mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::multiple) ? + nsAccessibleWrap::UnselectAll() : false; } //////////////////////////////////////////////////////////////////////////////// diff --git a/content/html/content/src/nsHTMLInputElement.cpp b/content/html/content/src/nsHTMLInputElement.cpp index dc49dc2167f4..5cda0681b970 100644 --- a/content/html/content/src/nsHTMLInputElement.cpp +++ b/content/html/content/src/nsHTMLInputElement.cpp @@ -303,9 +303,7 @@ AsyncClickHandler::Run() if (!filePicker) return NS_ERROR_FAILURE; - PRBool multi; - rv = mInput->GetMultiple(&multi); - NS_ENSURE_SUCCESS(rv, rv); + PRBool multi = mInput->HasAttr(kNameSpaceID_None, nsGkAtoms::multiple); rv = filePicker->Init(win, title, multi ? (PRInt16)nsIFilePicker::modeOpenMultiple : diff --git a/content/html/content/src/nsHTMLSelectElement.cpp b/content/html/content/src/nsHTMLSelectElement.cpp index bb1f1bb7b55c..fd2d006c8b48 100644 --- a/content/html/content/src/nsHTMLSelectElement.cpp +++ b/content/html/content/src/nsHTMLSelectElement.cpp @@ -293,9 +293,7 @@ nsHTMLSelectElement::InsertOptionsIntoList(nsIContent* aOptions, option->GetSelected(&selected); if (selected) { // Clear all other options - PRBool isMultiple; - GetMultiple(&isMultiple); - if (!isMultiple) { + if (!HasAttr(kNameSpaceID_None, nsGkAtoms::multiple)) { SetOptionsSelectedByIndex(i, i, PR_TRUE, PR_TRUE, PR_TRUE, PR_TRUE, nsnull); } @@ -701,9 +699,7 @@ nsHTMLSelectElement::GetOptions(nsIDOMHTMLOptionsCollection** aValue) NS_IMETHODIMP nsHTMLSelectElement::GetType(nsAString& aType) { - PRBool isMultiple; - GetMultiple(&isMultiple); - if (isMultiple) { + if (HasAttr(kNameSpaceID_None, nsGkAtoms::multiple)) { aType.AssignLiteral("select-multiple"); } else { @@ -933,11 +929,7 @@ nsHTMLSelectElement::SetOptionsSelectedByIndex(PRInt32 aStartIndex, } // First, find out whether multiple items can be selected - PRBool isMultiple; - rv = GetMultiple(&isMultiple); - if (NS_FAILED(rv)) { - isMultiple = PR_FALSE; - } + PRBool isMultiple = HasAttr(kNameSpaceID_None, nsGkAtoms::multiple); // These variables tell us whether any options were selected // or deselected. diff --git a/content/html/content/src/nsHTMLSelectElement.h b/content/html/content/src/nsHTMLSelectElement.h index 5860e5d89903..cb8bafc54a52 100644 --- a/content/html/content/src/nsHTMLSelectElement.h +++ b/content/html/content/src/nsHTMLSelectElement.h @@ -466,11 +466,13 @@ protected: * Is this a combobox? */ PRBool IsCombobox() { - PRBool isMultiple = PR_TRUE; + if (HasAttr(kNameSpaceID_None, nsGkAtoms::multiple)) { + return PR_FALSE; + } + PRInt32 size = 1; GetSize(&size); - GetMultiple(&isMultiple); - return !isMultiple && size <= 1; + return size <= 1; } /** diff --git a/layout/forms/nsListControlFrame.cpp b/layout/forms/nsListControlFrame.cpp index eead04e78303..eb244f9a3974 100644 --- a/layout/forms/nsListControlFrame.cpp +++ b/layout/forms/nsListControlFrame.cpp @@ -1189,27 +1189,6 @@ nsListControlFrame::Init(nsIContent* aContent, return result; } -PRBool -nsListControlFrame::GetMultiple(nsIDOMHTMLSelectElement* aSelect) const -{ - PRBool multiple = PR_FALSE; - nsresult rv = NS_OK; - if (aSelect) { - rv = aSelect->GetMultiple(&multiple); - } else { - nsCOMPtr selectElement = - do_QueryInterface(mContent); - - if (selectElement) { - rv = selectElement->GetMultiple(&multiple); - } - } - if (NS_SUCCEEDED(rv)) { - return multiple; - } - return PR_FALSE; -} - already_AddRefed nsListControlFrame::GetOptionAsContent(nsIDOMHTMLOptionsCollection* aCollection, PRInt32 aIndex) { diff --git a/layout/forms/nsListControlFrame.h b/layout/forms/nsListControlFrame.h index c6548ea95878..3ff966fdc6bc 100644 --- a/layout/forms/nsListControlFrame.h +++ b/layout/forms/nsListControlFrame.h @@ -297,10 +297,12 @@ protected: PRBool UpdateSelection(); /** - * Returns whether the nsIDOMHTMLSelectElement supports - * multiple selection. + * Returns whether mContent supports multiple selection. */ - PRBool GetMultiple(nsIDOMHTMLSelectElement* aSelect = nsnull) const; + PRBool GetMultiple() const { + return mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::multiple); + } + /** * Toggles (show/hide) the combobox dropdown menu.