diff --git a/dom/base/Document.h b/dom/base/Document.h index 8cd8269c449d..6c779db66834 100644 --- a/dom/base/Document.h +++ b/dom/base/Document.h @@ -606,7 +606,7 @@ class Document : public nsINode, aFocusedRadio, aRadioOut); } void AddToRadioGroup(const nsAString& aName, HTMLInputElement* aRadio) final { - DocumentOrShadowRoot::AddToRadioGroup(aName, aRadio, nullptr); + DocumentOrShadowRoot::AddToRadioGroup(aName, aRadio); } void RemoveFromRadioGroup(const nsAString& aName, HTMLInputElement* aRadio) final { diff --git a/dom/base/RadioGroupManager.cpp b/dom/base/RadioGroupManager.cpp index 759b58db0ea4..e7c8a730fae6 100644 --- a/dom/base/RadioGroupManager.cpp +++ b/dom/base/RadioGroupManager.cpp @@ -115,11 +115,9 @@ nsresult RadioGroupManager::GetNextRadioButton(const nsAString& aName, } void RadioGroupManager::AddToRadioGroup(const nsAString& aName, - HTMLInputElement* aRadio, - nsIContent* aAncestor) { + HTMLInputElement* aRadio) { nsRadioGroupStruct* radioGroup = GetOrCreateRadioGroup(aName); - nsContentUtils::AddElementToListByTreeOrder(radioGroup->mRadioButtons, aRadio, - aAncestor); + radioGroup->mRadioButtons.AppendElement(aRadio); if (aRadio->IsRequired()) { radioGroup->mRequiredRadioCount++; diff --git a/dom/base/RadioGroupManager.h b/dom/base/RadioGroupManager.h index 1119462a7d98..f25bf5e7539d 100644 --- a/dom/base/RadioGroupManager.h +++ b/dom/base/RadioGroupManager.h @@ -12,8 +12,6 @@ #include "nsIRadioGroupContainer.h" #include "nsClassHashtable.h" -class nsIContent; - namespace mozilla { namespace html { @@ -39,8 +37,7 @@ class RadioGroupManager { nsresult GetNextRadioButton(const nsAString& aName, const bool aPrevious, HTMLInputElement* aFocusedRadio, HTMLInputElement** aRadioOut); - void AddToRadioGroup(const nsAString& aName, HTMLInputElement* aRadio, - nsIContent* aAncestor); + void AddToRadioGroup(const nsAString& aName, HTMLInputElement* aRadio); void RemoveFromRadioGroup(const nsAString& aName, HTMLInputElement* aRadio); uint32_t GetRequiredRadioCount(const nsAString& aName) const; void RadioRequiredWillChange(const nsAString& aName, bool aRequiredAdded); diff --git a/dom/base/ShadowRoot.h b/dom/base/ShadowRoot.h index aceb3ab56a73..d0678e3e3df7 100644 --- a/dom/base/ShadowRoot.h +++ b/dom/base/ShadowRoot.h @@ -255,7 +255,7 @@ class ShadowRoot final : public DocumentFragment, } void AddToRadioGroup(const nsAString& aName, HTMLInputElement* aRadio) override { - DocumentOrShadowRoot::AddToRadioGroup(aName, aRadio, this); + DocumentOrShadowRoot::AddToRadioGroup(aName, aRadio); } void RemoveFromRadioGroup(const nsAString& aName, HTMLInputElement* aRadio) override { diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index 7292d41ed4d3..86b69470a294 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -175,7 +175,6 @@ #include "mozilla/dom/FromParser.h" #include "mozilla/dom/HTMLElement.h" #include "mozilla/dom/HTMLFormElement.h" -#include "mozilla/dom/HTMLImageElement.h" #include "mozilla/dom/HTMLInputElement.h" #include "mozilla/dom/HTMLTextAreaElement.h" #include "mozilla/dom/IPCBlob.h" @@ -11226,84 +11225,6 @@ nsIContent* nsContentUtils::GetClosestLinkInFlatTree(nsIContent* aContent) { return nullptr; } -namespace { - -struct TreePositionComparator { - Element* const mChild; - nsIContent* const mAncestor; - TreePositionComparator(Element* aChild, nsIContent* aAncestor) - : mChild(aChild), mAncestor(aAncestor) {} - int operator()(Element* aElement) const { - return nsLayoutUtils::CompareTreePosition(mChild, aElement, mAncestor); - } -}; - -} // namespace - -/* static */ -int32_t nsContentUtils::CompareTreePosition(nsIContent* aContent1, - nsIContent* aContent2, - const nsIContent* aCommonAncestor) { - NS_ASSERTION(aContent1 != aContent2, "Comparing content to itself"); - - // TODO: remove the prevent asserts fix, see bug 598468. -#ifdef DEBUG - nsLayoutUtils::gPreventAssertInCompareTreePosition = true; - int32_t rVal = - nsLayoutUtils::CompareTreePosition(aContent1, aContent2, aCommonAncestor); - nsLayoutUtils::gPreventAssertInCompareTreePosition = false; - - return rVal; -#else // DEBUG - return nsLayoutUtils::CompareTreePosition(aContent1, aContent2, - aCommonAncestor); -#endif // DEBUG -} - -/* static */ -template -bool nsContentUtils::AddElementToListByTreeOrder(nsTArray& aList, - ElementPtr aChild, - nsIContent* aCommonAncestor) { - NS_ASSERTION(aList.IndexOf(aChild) == aList.NoIndex, - "aChild already in aList"); - - const uint32_t count = aList.Length(); - ElementType element; - - // Optimize most common case where we insert at the end. - int32_t position = -1; - if (count > 0) { - element = aList[count - 1]; - position = CompareTreePosition(aChild, element, aCommonAncestor); - } - - // If this item comes after the last element, or the elements array is - // empty, we append to the end. Otherwise, we do a binary search to - // determine where the element should go. - if (position >= 0 || count == 0) { - aList.AppendElement(aChild); - return true; - } - - size_t idx; - BinarySearchIf(aList, 0, count, - TreePositionComparator(aChild, aCommonAncestor), &idx); - - aList.InsertElementAt(idx, aChild); - return false; -} - -template bool nsContentUtils::AddElementToListByTreeOrder( - nsTArray& aList, - nsGenericHTMLFormElement* aChild, nsIContent* aAncestor); -template bool nsContentUtils::AddElementToListByTreeOrder( - nsTArray& aList, HTMLImageElement* aChild, - nsIContent* aAncestor); -template bool nsContentUtils::AddElementToListByTreeOrder( - nsTArray>& aList, HTMLInputElement* aChild, - nsIContent* aAncestor); - namespace mozilla { std::ostream& operator<<(std::ostream& aOut, const PreventDefaultResult aPreventDefaultResult) { diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h index 65f9a8c30018..2f3d5d81ef2c 100644 --- a/dom/base/nsContentUtils.h +++ b/dom/base/nsContentUtils.h @@ -3435,33 +3435,6 @@ class nsContentUtils { static bool IsExternalProtocol(nsIURI* aURI); - /** - * Add an element to a list, keeping the list sorted by tree order. - * Can take a potential ancestor of the elements in order to speed up - * tree-order comparisons, if such an ancestor exists. - * Returns true if the element is appended to the end of the list. - */ - template - static bool AddElementToListByTreeOrder(nsTArray& aList, - ElementPtr aChild, - nsIContent* aCommonAncestor); - - /** - * Compares the position of aContent1 and aContent2 in the document - * @param aContent1 First content to compare. - * @param aContent2 Second content to compare. - * @param aCommonAncestor Potential ancestor of the contents, if one exists. - * This is only a hint; if it's not an ancestor of - * aContent1 or aContent2, this function will still - * work, but it will be slower than normal. - * @return < 0 if aContent1 is before aContent2, - * > 0 if aContent1 is after aContent2, - * 0 otherwise - */ - static int32_t CompareTreePosition(nsIContent* aContent1, - nsIContent* aContent2, - const nsIContent* aCommonAncestor); - private: static bool InitializeEventTable(); diff --git a/dom/html/HTMLFormControlsCollection.cpp b/dom/html/HTMLFormControlsCollection.cpp index 406a00a1ea7c..bf25882e05c4 100644 --- a/dom/html/HTMLFormControlsCollection.cpp +++ b/dom/html/HTMLFormControlsCollection.cpp @@ -222,9 +222,9 @@ nsresult HTMLFormControlsCollection::GetSortedControls( // Determine which of the two elements should be ordered // first and add it to the end of the list. nsGenericHTMLFormElement* elementToAdd; - if (nsContentUtils::CompareTreePosition(mElements[elementsIdx], - mNotInElements[notInElementsIdx], - mForm) < 0) { + if (HTMLFormElement::CompareFormControlPosition( + mElements[elementsIdx], mNotInElements[notInElementsIdx], mForm) < + 0) { elementToAdd = mElements[elementsIdx]; ++elementsIdx; } else { diff --git a/dom/html/HTMLFormElement.cpp b/dom/html/HTMLFormElement.cpp index 560ee5ff0708..e30f1e58161e 100644 --- a/dom/html/HTMLFormElement.cpp +++ b/dom/html/HTMLFormElement.cpp @@ -1086,6 +1086,44 @@ Element* HTMLFormElement::IndexedGetter(uint32_t aIndex, bool& aFound) { return element; } +/** + * Compares the position of aControl1 and aControl2 in the document + * @param aControl1 First control to compare. + * @param aControl2 Second control to compare. + * @param aForm Parent form of the controls. + * @return < 0 if aControl1 is before aControl2, + * > 0 if aControl1 is after aControl2, + * 0 otherwise + */ +/* static */ +int32_t HTMLFormElement::CompareFormControlPosition(Element* aElement1, + Element* aElement2, + const nsIContent* aForm) { + NS_ASSERTION(aElement1 != aElement2, "Comparing a form control to itself"); + + // If an element has a @form, we can assume it *might* be able to not have + // a parent and still be in the form. + NS_ASSERTION( + (aElement1->HasAttr(nsGkAtoms::form) || aElement1->GetParent()) && + (aElement2->HasAttr(nsGkAtoms::form) || aElement2->GetParent()), + "Form controls should always have parents"); + + // If we pass aForm, we are assuming both controls are form descendants which + // is not always the case. This function should work but maybe slower. + // However, checking if both elements are form descendants may be slow too... + // TODO: remove the prevent asserts fix, see bug 598468. +#ifdef DEBUG + nsLayoutUtils::gPreventAssertInCompareTreePosition = true; + int32_t rVal = + nsLayoutUtils::CompareTreePosition(aElement1, aElement2, aForm); + nsLayoutUtils::gPreventAssertInCompareTreePosition = false; + + return rVal; +#else // DEBUG + return nsLayoutUtils::CompareTreePosition(aElement1, aElement2, aForm); +#endif // DEBUG +} + #ifdef DEBUG /** * Checks that all form elements are in document order. Asserts if any pair of @@ -1168,6 +1206,58 @@ void HTMLFormElement::PostPossibleUsernameEvent() { mFormPossibleUsernameEventDispatcher->PostDOMEvent(); } +namespace { + +struct FormComparator { + Element* const mChild; + HTMLFormElement* const mForm; + FormComparator(Element* aChild, HTMLFormElement* aForm) + : mChild(aChild), mForm(aForm) {} + int operator()(Element* aElement) const { + return HTMLFormElement::CompareFormControlPosition(mChild, aElement, mForm); + } +}; + +} // namespace + +// This function return true if the element, once appended, is the last one in +// the array. +template +static bool AddElementToList(nsTArray& aList, ElementType* aChild, + HTMLFormElement* aForm) { + NS_ASSERTION(aList.IndexOf(aChild) == aList.NoIndex, + "aChild already in aList"); + + const uint32_t count = aList.Length(); + ElementType* element; + bool lastElement = false; + + // Optimize most common case where we insert at the end. + int32_t position = -1; + if (count > 0) { + element = aList[count - 1]; + position = + HTMLFormElement::CompareFormControlPosition(aChild, element, aForm); + } + + // If this item comes after the last element, or the elements array is + // empty, we append to the end. Otherwise, we do a binary search to + // determine where the element should go. + if (position >= 0 || count == 0) { + // WEAK - don't addref + aList.AppendElement(aChild); + lastElement = true; + } else { + size_t idx; + BinarySearchIf(aList, 0, count, FormComparator(aChild, aForm), &idx); + + // WEAK - don't addref + aList.InsertElementAt(idx, aChild); + } + + return lastElement; +} + nsresult HTMLFormElement::AddElement(nsGenericHTMLFormElement* aChild, bool aUpdateValidity, bool aNotify) { // If an element has a @form, we can assume it *might* be able to not have @@ -1182,8 +1272,7 @@ nsresult HTMLFormElement::AddElement(nsGenericHTMLFormElement* aChild, nsTArray& controlList = childInElements ? mControls->mElements : mControls->mNotInElements; - bool lastElement = - nsContentUtils::AddElementToListByTreeOrder(controlList, aChild, this); + bool lastElement = AddElementToList(controlList, aChild, this); #ifdef DEBUG AssertDocumentOrder(controlList, this); @@ -1219,16 +1308,16 @@ nsresult HTMLFormElement::AddElement(nsGenericHTMLFormElement* aChild, // what's in the slot or the child is earlier than the default submit. nsGenericHTMLFormElement* oldDefaultSubmit = mDefaultSubmitElement; if (!*firstSubmitSlot || - (!lastElement && nsContentUtils::CompareTreePosition( - aChild, *firstSubmitSlot, this) < 0)) { + (!lastElement && + CompareFormControlPosition(aChild, *firstSubmitSlot, this) < 0)) { // Update mDefaultSubmitElement if it's currently in a valid state. // Valid state means either non-null or null because there are in fact // no submit elements around. if ((mDefaultSubmitElement || (!mFirstSubmitInElements && !mFirstSubmitNotInElements)) && (*firstSubmitSlot == mDefaultSubmitElement || - nsContentUtils::CompareTreePosition(aChild, mDefaultSubmitElement, - this) < 0)) { + CompareFormControlPosition(aChild, mDefaultSubmitElement, this) < + 0)) { mDefaultSubmitElement = aChild; } *firstSubmitSlot = aChild; @@ -1359,8 +1448,8 @@ void HTMLFormElement::HandleDefaultSubmitRemoval() { "How did that happen?"); // Have both; use the earlier one mDefaultSubmitElement = - nsContentUtils::CompareTreePosition(mFirstSubmitInElements, - mFirstSubmitNotInElements, this) < 0 + CompareFormControlPosition(mFirstSubmitInElements, + mFirstSubmitNotInElements, this) < 0 ? mFirstSubmitInElements : mFirstSubmitNotInElements; } @@ -1708,8 +1797,8 @@ bool HTMLFormElement::IsDefaultSubmitElement( // We have both kinds of submits. Check which comes first. nsGenericHTMLFormElement* defaultSubmit = - nsContentUtils::CompareTreePosition(mFirstSubmitInElements, - mFirstSubmitNotInElements, this) < 0 + CompareFormControlPosition(mFirstSubmitInElements, + mFirstSubmitNotInElements, this) < 0 ? mFirstSubmitInElements : mFirstSubmitNotInElements; return aElement == defaultSubmit; @@ -1922,7 +2011,7 @@ HTMLFormElement::WalkRadioGroup(const nsAString& aName, void HTMLFormElement::AddToRadioGroup(const nsAString& aName, HTMLInputElement* aRadio) { - RadioGroupManager::AddToRadioGroup(aName, aRadio, this); + RadioGroupManager::AddToRadioGroup(aName, aRadio); } void HTMLFormElement::RemoveFromRadioGroup(const nsAString& aName, @@ -2078,7 +2167,7 @@ nsresult HTMLFormElement::AddElementToTableInternal( } nsresult HTMLFormElement::AddImageElement(HTMLImageElement* aChild) { - nsContentUtils::AddElementToListByTreeOrder(mImageElements, aChild, this); + AddElementToList(mImageElements, aChild, this); return NS_OK; } diff --git a/dom/html/HTMLFormElement.h b/dom/html/HTMLFormElement.h index 6491b8510ac6..80b40c6bcee0 100644 --- a/dom/html/HTMLFormElement.h +++ b/dom/html/HTMLFormElement.h @@ -379,6 +379,9 @@ class HTMLFormElement final : public nsGenericHTMLElement, void GetSupportedNames(nsTArray& aRetval); + static int32_t CompareFormControlPosition(Element* aElement1, + Element* aElement2, + const nsIContent* aForm); #ifdef DEBUG static void AssertDocumentOrder( const nsTArray& aControls, nsIContent* aForm); diff --git a/dom/html/test/forms/mochitest.ini b/dom/html/test/forms/mochitest.ini index 111d34f2cbea..2d84628b5f49 100644 --- a/dom/html/test/forms/mochitest.ini +++ b/dom/html/test/forms/mochitest.ini @@ -14,7 +14,6 @@ prefs = [test_bug1286509.html] [test_button_attributes_reflection.html] [test_input_radio_indeterminate.html] -[test_input_radio_navigation_order.html] [test_input_radio_radiogroup.html] [test_input_radio_required.html] [test_change_event.html] diff --git a/testing/web-platform/tests/html/semantics/forms/the-input-element/radio-keyboard-navigation-order.html b/testing/web-platform/tests/html/semantics/forms/the-input-element/radio-keyboard-navigation-order.html deleted file mode 100644 index d019ca982c8d..000000000000 --- a/testing/web-platform/tests/html/semantics/forms/the-input-element/radio-keyboard-navigation-order.html +++ /dev/null @@ -1,70 +0,0 @@ - - - - -Radio button group keyboard navigation order - - - - - - - -
- - - -
-
- - - - - - -
- -
- - - - - - -