From 1b3514dbdfcf40860b931547312f10f0084088e3 Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Mon, 17 Jun 2013 13:07:04 -0400 Subject: [PATCH] Bug 870787 - Improve named getter for form, r=bz --- content/base/public/nsContentUtils.h | 11 - content/base/src/nsContentList.cpp | 20 - content/base/src/nsContentList.h | 10 - content/base/src/nsContentUtils.cpp | 56 --- content/base/src/nsNodeUtils.cpp | 6 + content/html/content/src/HTMLImageElement.cpp | 131 ++++++- content/html/content/src/HTMLImageElement.h | 18 + .../html/content/src/nsGenericHTMLElement.h | 15 +- .../html/content/src/nsHTMLFormElement.cpp | 365 ++++++++++++------ content/html/content/src/nsHTMLFormElement.h | 69 +++- content/html/content/test/Makefile.in | 1 + content/html/content/test/test_bug870787.html | 84 ++++ content/html/document/src/nsHTMLDocument.cpp | 33 -- content/html/document/src/nsHTMLDocument.h | 3 - content/html/document/src/nsIHTMLDocument.h | 4 - 15 files changed, 560 insertions(+), 266 deletions(-) create mode 100644 content/html/content/test/test_bug870787.html diff --git a/content/base/public/nsContentUtils.h b/content/base/public/nsContentUtils.h index 4977d417826..2bf54e8b46f 100644 --- a/content/base/public/nsContentUtils.h +++ b/content/base/public/nsContentUtils.h @@ -510,17 +510,6 @@ public: static nsresult GuessCharset(const char *aData, uint32_t aDataLen, nsACString &aCharset); - /** - * Determine whether aContent is in some way associated with aForm. If the - * form is a container the only elements that are considered to be associated - * with a form are the elements that are contained within the form. If the - * form is a leaf element then all elements will be accepted into this list, - * since this can happen due to content fixup when a form spans table rows or - * table cells. - */ - static bool BelongsInForm(nsIContent *aForm, - nsIContent *aContent); - static nsresult CheckQName(const nsAString& aQualifiedName, bool aNamespaceAware = true, const PRUnichar** aColon = nullptr); diff --git a/content/base/src/nsContentList.cpp b/content/base/src/nsContentList.cpp index b691ec6c0d5..ea435f19bdf 100644 --- a/content/base/src/nsContentList.cpp +++ b/content/base/src/nsContentList.cpp @@ -140,26 +140,6 @@ nsSimpleContentList::WrapObject(JSContext *cx, JS::Handle scope) return NodeListBinding::Wrap(cx, scope, this); } -// nsFormContentList - -nsFormContentList::nsFormContentList(nsIContent *aForm, - nsBaseContentList& aContentList) - : nsSimpleContentList(aForm) -{ - - // move elements that belong to mForm into this content list - - uint32_t i, length = 0; - aContentList.GetLength(&length); - - for (i = 0; i < length; i++) { - nsIContent *c = aContentList.Item(i); - if (c && nsContentUtils::BelongsInForm(aForm, c)) { - AppendElement(c); - } - } -} - // Hashtable for storing nsContentLists static PLDHashTable gContentListHashTable; diff --git a/content/base/src/nsContentList.h b/content/base/src/nsContentList.h index 2b97ac96ad6..ace8b16379a 100644 --- a/content/base/src/nsContentList.h +++ b/content/base/src/nsContentList.h @@ -124,16 +124,6 @@ private: nsCOMPtr mRoot; }; -// This class is used only by form element code and this is a static -// list of elements. NOTE! This list holds strong references to -// the elements in the list. -class nsFormContentList : public nsSimpleContentList -{ -public: - nsFormContentList(nsIContent *aForm, - nsBaseContentList& aContentList); -}; - /** * Class that's used as the key to hash nsContentList implementations * for fast retrieval diff --git a/content/base/src/nsContentUtils.cpp b/content/base/src/nsContentUtils.cpp index a027efc0cd6..53e54eb3f00 100644 --- a/content/base/src/nsContentUtils.cpp +++ b/content/base/src/nsContentUtils.cpp @@ -2365,62 +2365,6 @@ nsContentUtils::NewURIWithDocumentCharset(nsIURI** aResult, aBaseURI, sIOService); } -// static -bool -nsContentUtils::BelongsInForm(nsIContent *aForm, - nsIContent *aContent) -{ - NS_PRECONDITION(aForm, "Must have a form"); - NS_PRECONDITION(aContent, "Must have a content node"); - - if (aForm == aContent) { - // A form does not belong inside itself, so we return false here - - return false; - } - - nsIContent* content = aContent->GetParent(); - - while (content) { - if (content == aForm) { - // aContent is contained within the form so we return true. - - return true; - } - - if (content->Tag() == nsGkAtoms::form && - content->IsHTML()) { - // The child is contained within a form, but not the right form - // so we ignore it. - - return false; - } - - content = content->GetParent(); - } - - if (aForm->GetChildCount() > 0) { - // The form is a container but aContent wasn't inside the form, - // return false - - return false; - } - - // The form is a leaf and aContent wasn't inside any other form so - // we check whether the content comes after the form. If it does, - // return true. If it does not, then it couldn't have been inside - // the form in the HTML. - if (PositionIsBefore(aForm, aContent)) { - // We could be in this form! - // In the future, we may want to get document.forms, look at the - // form after aForm, and if aContent is after that form after - // aForm return false here.... - return true; - } - - return false; -} - // static nsresult nsContentUtils::CheckQName(const nsAString& aQualifiedName, diff --git a/content/base/src/nsNodeUtils.cpp b/content/base/src/nsNodeUtils.cpp index 15d926a034d..211d25b8b16 100644 --- a/content/base/src/nsNodeUtils.cpp +++ b/content/base/src/nsNodeUtils.cpp @@ -25,6 +25,7 @@ #endif #include "nsBindingManager.h" #include "nsGenericHTMLElement.h" +#include "mozilla/dom/HTMLImageElement.h" #include "mozilla/dom/HTMLMediaElement.h" #include "nsWrapperCacheInlines.h" #include "nsObjectLoadingContent.h" @@ -216,6 +217,11 @@ nsNodeUtils::LastRelease(nsINode* aNode) // notify, since we're being destroyed in any case. static_cast(aNode)->ClearForm(true); } + + if (aNode->IsElement() && aNode->AsElement()->IsHTML(nsGkAtoms::img)) { + HTMLImageElement* imageElem = static_cast(aNode); + imageElem->ClearForm(true); + } } aNode->UnsetFlags(NODE_HAS_PROPERTIES); diff --git a/content/html/content/src/HTMLImageElement.cpp b/content/html/content/src/HTMLImageElement.cpp index 0940ffb6bfc..8bff2e39b70 100644 --- a/content/html/content/src/HTMLImageElement.cpp +++ b/content/html/content/src/HTMLImageElement.cpp @@ -25,6 +25,7 @@ #include "nsContentPolicyUtils.h" #include "nsIDOMWindow.h" #include "nsFocusManager.h" +#include "nsHTMLFormElement.h" #include "imgIContainer.h" #include "imgILoader.h" @@ -67,6 +68,7 @@ namespace dom { HTMLImageElement::HTMLImageElement(already_AddRefed aNodeInfo) : nsGenericHTMLElement(aNodeInfo) + , mForm(nullptr) { // We start out broken AddStatesSilently(NS_EVENT_STATE_BROKEN); @@ -84,7 +86,7 @@ NS_IMPL_RELEASE_INHERITED(HTMLImageElement, Element) // QueryInterface implementation for HTMLImageElement -NS_INTERFACE_TABLE_HEAD(HTMLImageElement) +NS_INTERFACE_TABLE_HEAD_CYCLE_COLLECTION_INHERITED(HTMLImageElement) NS_HTML_CONTENT_INTERFACES(nsGenericHTMLElement) NS_INTERFACE_TABLE_INHERITED4(HTMLImageElement, nsIDOMHTMLImageElement, @@ -296,6 +298,46 @@ HTMLImageElement::GetAttributeMappingFunction() const } +nsresult +HTMLImageElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue, + bool aNotify) +{ + + if (aNameSpaceID == kNameSpaceID_None && mForm && + (aName == nsGkAtoms::name || aName == nsGkAtoms::id)) { + // remove the image from the hashtable as needed + nsAutoString tmp; + GetAttr(kNameSpaceID_None, aName, tmp); + + if (!tmp.IsEmpty()) { + mForm->RemoveImageElementFromTable(this, tmp); + } + } + + return nsGenericHTMLElement::BeforeSetAttr(aNameSpaceID, aName, + aValue, aNotify); +} + +nsresult +HTMLImageElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, + const nsAttrValue* aValue, bool aNotify) +{ + if (aNameSpaceID == kNameSpaceID_None && mForm && + (aName == nsGkAtoms::name || aName == nsGkAtoms::id) && + aValue && !aValue->IsEmptyString()) { + // add the image to the hashtable as needed + NS_ABORT_IF_FALSE(aValue->Type() == nsAttrValue::eAtom, + "Expected atom value for name/id"); + mForm->AddImageElementToTable(this, + nsDependentAtomString(aValue->GetAtomValue())); + } + + return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, + aValue, aNotify); +} + + nsresult HTMLImageElement::PreHandleEvent(nsEventChainPreVisitor& aVisitor) { @@ -414,6 +456,10 @@ HTMLImageElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent, nsImageLoadingContent::BindToTree(aDocument, aParent, aBindingParent, aCompileEventHandlers); + if (aParent) { + UpdateFormOwner(); + } + if (HasAttr(kNameSpaceID_None, nsGkAtoms::src)) { // FIXME: Bug 660963 it would be nice if we could just have // ClearBrokenState update our state and do it fast... @@ -434,10 +480,45 @@ HTMLImageElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent, void HTMLImageElement::UnbindFromTree(bool aDeep, bool aNullParent) { + if (mForm) { + if (aNullParent || !FindAncestorForm(mForm)) { + ClearForm(true); + } else { + UnsetFlags(MAYBE_ORPHAN_FORM_ELEMENT); + } + } + nsImageLoadingContent::UnbindFromTree(aDeep, aNullParent); nsGenericHTMLElement::UnbindFromTree(aDeep, aNullParent); } +void +HTMLImageElement::UpdateFormOwner() +{ + if (!mForm) { + mForm = FindAncestorForm(); + } + + if (mForm && !HasFlag(ADDED_TO_FORM)) { + // Now we need to add ourselves to the form + nsAutoString nameVal, idVal; + GetAttr(kNameSpaceID_None, nsGkAtoms::name, nameVal); + GetAttr(kNameSpaceID_None, nsGkAtoms::id, idVal); + + SetFlags(ADDED_TO_FORM); + + mForm->AddImageElement(this); + + if (!nameVal.IsEmpty()) { + mForm->AddImageElementToTable(this, nameVal); + } + + if (!idVal.IsEmpty()) { + mForm->AddImageElementToTable(this, idVal); + } + } +} + void HTMLImageElement::MaybeLoadImage() { @@ -577,5 +658,53 @@ HTMLImageElement::WrapNode(JSContext* aCx, JS::Handle aScope) return HTMLImageElementBinding::Wrap(aCx, aScope, this); } +#ifdef DEBUG +nsIDOMHTMLFormElement* +HTMLImageElement::GetForm() const +{ + return mForm; +} +#endif + +void +HTMLImageElement::SetForm(nsIDOMHTMLFormElement* aForm) +{ + NS_PRECONDITION(aForm, "Don't pass null here"); + NS_ASSERTION(!mForm, + "We don't support switching from one non-null form to another."); + + mForm = static_cast(aForm); +} + +void +HTMLImageElement::ClearForm(bool aRemoveFromForm) +{ + NS_ASSERTION((mForm != nullptr) == HasFlag(ADDED_TO_FORM), + "Form control should have had flag set correctly"); + + if (!mForm) { + return; + } + + if (aRemoveFromForm) { + nsAutoString nameVal, idVal; + GetAttr(kNameSpaceID_None, nsGkAtoms::name, nameVal); + GetAttr(kNameSpaceID_None, nsGkAtoms::id, idVal); + + mForm->RemoveImageElement(this); + + if (!nameVal.IsEmpty()) { + mForm->RemoveImageElementFromTable(this, nameVal); + } + + if (!idVal.IsEmpty()) { + mForm->RemoveImageElementFromTable(this, idVal); + } + } + + UnsetFlags(ADDED_TO_FORM); + mForm = nullptr; +} + } // namespace dom } // namespace mozilla diff --git a/content/html/content/src/HTMLImageElement.h b/content/html/content/src/HTMLImageElement.h index 5300b3c0d0a..262963dc918 100644 --- a/content/html/content/src/HTMLImageElement.h +++ b/content/html/content/src/HTMLImageElement.h @@ -174,12 +174,30 @@ public: SetHTMLAttr(nsGkAtoms::lowsrc, aLowsrc, aError); } +#ifdef DEBUG + nsIDOMHTMLFormElement* GetForm() const; +#endif + void SetForm(nsIDOMHTMLFormElement* aForm); + void ClearForm(bool aRemoveFromForm); + protected: CSSIntPoint GetXY(); virtual void GetItemValueText(nsAString& text) MOZ_OVERRIDE; virtual void SetItemValueText(const nsAString& text) MOZ_OVERRIDE; virtual JSObject* WrapNode(JSContext *aCx, JS::Handle aScope) MOZ_OVERRIDE; + void UpdateFormOwner(); + + virtual nsresult BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue, + bool aNotify) MOZ_OVERRIDE; + + virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, + const nsAttrValue* aValue, bool aNotify) MOZ_OVERRIDE; + + // This is a weak reference that this element and the HTMLFormElement + // cooperate in maintaining. + nsHTMLFormElement* mForm; }; } // namespace dom diff --git a/content/html/content/src/nsGenericHTMLElement.h b/content/html/content/src/nsGenericHTMLElement.h index f8251352ac2..e3d41b3fd45 100644 --- a/content/html/content/src/nsGenericHTMLElement.h +++ b/content/html/content/src/nsGenericHTMLElement.h @@ -1047,15 +1047,16 @@ class HTMLFieldSetElement; // Form element specific bits enum { - // If this flag is set on an nsGenericHTMLFormElement, that means that we have - // added ourselves to our mForm. It's possible to have a non-null mForm, but - // not have this flag set. That happens when the form is set via the content - // sink. + // If this flag is set on an nsGenericHTMLFormElement or an HTMLImageElement, + // that means that we have added ourselves to our mForm. It's possible to + // have a non-null mForm, but not have this flag set. That happens when the + // form is set via the content sink. ADDED_TO_FORM = FORM_ELEMENT_FLAG_BIT(0), - // If this flag is set on an nsGenericHTMLFormElement, that means that its form - // is in the process of being unbound from the tree, and this form element - // hasn't re-found its form in nsGenericHTMLFormElement::UnbindFromTree yet. + // If this flag is set on an nsGenericHTMLFormElement or an HTMLImageElement, + // that means that its form is in the process of being unbound from the tree, + // and this form element hasn't re-found its form in + // nsGenericHTMLFormElement::UnbindFromTree yet. MAYBE_ORPHAN_FORM_ELEMENT = FORM_ELEMENT_FLAG_BIT(1) }; diff --git a/content/html/content/src/nsHTMLFormElement.cpp b/content/html/content/src/nsHTMLFormElement.cpp index 314d689e453..4b9c2a173d5 100644 --- a/content/html/content/src/nsHTMLFormElement.cpp +++ b/content/html/content/src/nsHTMLFormElement.cpp @@ -54,6 +54,9 @@ #include "mozilla/dom/BindingUtils.h" #include "nsSandboxFlags.h" +// images +#include "mozilla/dom/HTMLImageElement.h" + using namespace mozilla::dom; static const int NS_FORM_CONTROL_LIST_HASHTABLE_SIZE = 16; @@ -104,6 +107,8 @@ public: nsresult AddElementToTable(nsGenericHTMLFormElement* aChild, const nsAString& aName); + nsresult AddImageElementToTable(HTMLImageElement* aChild, + const nsAString& aName); nsresult RemoveElementFromTable(nsGenericHTMLFormElement* aChild, const nsAString& aName); nsresult IndexOfControl(nsIFormControl* aControl, @@ -241,6 +246,7 @@ nsHTMLFormElement::nsHTMLFormElement(already_AddRefed aNodeInfo) mInvalidElementsCount(0), mEverTriedInvalidSubmit(false) { + mImageNameLookupTable.Init(NS_FORM_CONTROL_LIST_HASHTABLE_SIZE); } nsHTMLFormElement::~nsHTMLFormElement() @@ -248,6 +254,8 @@ nsHTMLFormElement::~nsHTMLFormElement() if (mControls) { mControls->DropFormReference(); } + + Clear(); } nsresult @@ -290,9 +298,15 @@ ElementTraverser(const nsAString& key, nsIDOMHTMLInputElement* element, NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(nsHTMLFormElement, nsGenericHTMLElement) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mControls) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mImageNameLookupTable) tmp->mSelectedRadioButtons.EnumerateRead(ElementTraverser, &cb); NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END +NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(nsHTMLFormElement, + nsGenericHTMLElement) + tmp->Clear(); +NS_IMPL_CYCLE_COLLECTION_UNLINK_END + NS_IMPL_ADDREF_INHERITED(nsHTMLFormElement, Element) NS_IMPL_RELEASE_INHERITED(nsHTMLFormElement, Element) @@ -455,8 +469,9 @@ nsHTMLFormElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent, return rv; } +template static void -MarkOrphans(const nsTArray& aArray) +MarkOrphans(const nsTArray& aArray) { uint32_t length = aArray.Length(); for (uint32_t i = 0; i < length; ++i) { @@ -483,8 +498,7 @@ CollectOrphans(nsINode* aRemovalRoot, // Now if MAYBE_ORPHAN_FORM_ELEMENT is not set, that would mean that the // node is in fact a descendant of the form and hence should stay in the // form. If it _is_ set, then we need to check whether the node is a - // descendant of aRemovalRoot. If it is, we leave it in the form. See - // also the code in nsGenericHTMLFormElement::FindForm. + // descendant of aRemovalRoot. If it is, we leave it in the form. #ifdef DEBUG bool removed = false; #endif @@ -511,6 +525,46 @@ CollectOrphans(nsINode* aRemovalRoot, } } +static void +CollectOrphans(nsINode* aRemovalRoot, + const nsTArray& aArray +#ifdef DEBUG + , nsIDOMHTMLFormElement* aThisForm +#endif + ) +{ + // Walk backwards so that if we remove elements we can just keep iterating + uint32_t length = aArray.Length(); + for (uint32_t i = length; i > 0; --i) { + HTMLImageElement* node = aArray[i-1]; + + // Now if MAYBE_ORPHAN_FORM_ELEMENT is not set, that would mean that the + // node is in fact a descendant of the form and hence should stay in the + // form. If it _is_ set, then we need to check whether the node is a + // descendant of aRemovalRoot. If it is, we leave it in the form. +#ifdef DEBUG + bool removed = false; +#endif + if (node->HasFlag(MAYBE_ORPHAN_FORM_ELEMENT)) { + node->UnsetFlags(MAYBE_ORPHAN_FORM_ELEMENT); + if (!nsContentUtils::ContentIsDescendantOf(node, aRemovalRoot)) { + node->ClearForm(true); + +#ifdef DEBUG + removed = true; +#endif + } + } + +#ifdef DEBUG + if (!removed) { + nsCOMPtr form = node->GetForm(); + NS_ASSERTION(form == aThisForm, "How did that happen?"); + } +#endif /* DEBUG */ + } +} + void nsHTMLFormElement::UnbindFromTree(bool aDeep, bool aNullParent) { @@ -519,6 +573,7 @@ nsHTMLFormElement::UnbindFromTree(bool aDeep, bool aNullParent) // Mark all of our controls as maybe being orphans MarkOrphans(mControls->mElements); MarkOrphans(mControls->mNotInElements); + MarkOrphans(mImageElements); nsGenericHTMLElement::UnbindFromTree(aDeep, aNullParent); @@ -535,17 +590,22 @@ nsHTMLFormElement::UnbindFromTree(bool aDeep, bool aNullParent) CollectOrphans(ancestor, mControls->mElements #ifdef DEBUG , this -#endif +#endif ); CollectOrphans(ancestor, mControls->mNotInElements #ifdef DEBUG , this -#endif +#endif + ); + CollectOrphans(ancestor, mImageElements +#ifdef DEBUG + , this +#endif ); if (oldDocument) { oldDocument->RemovedForm(); - } + } ForgetCurrentSubmission(); } @@ -1034,18 +1094,17 @@ nsHTMLFormElement::GetElementAt(int32_t aIndex) const * 0 otherwise */ static inline int32_t -CompareFormControlPosition(nsGenericHTMLFormElement *aControl1, - nsGenericHTMLFormElement *aControl2, +CompareFormControlPosition(Element *aElement1, Element *aElement2, const nsIContent* aForm) { - NS_ASSERTION(aControl1 != aControl2, "Comparing a form control to itself"); + 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((aControl1->HasAttr(kNameSpaceID_None, nsGkAtoms::form) || - aControl1->GetParent()) && - (aControl2->HasAttr(kNameSpaceID_None, nsGkAtoms::form) || - aControl2->GetParent()), + NS_ASSERTION((aElement1->HasAttr(kNameSpaceID_None, nsGkAtoms::form) || + aElement1->GetParent()) && + (aElement2->HasAttr(kNameSpaceID_None, nsGkAtoms::form) || + aElement2->GetParent()), "Form controls should always have parents"); // If we pass aForm, we are assuming both controls are form descendants which @@ -1054,15 +1113,15 @@ CompareFormControlPosition(nsGenericHTMLFormElement *aControl1, // TODO: remove the prevent asserts fix, see bug 598468. #ifdef DEBUG nsLayoutUtils::gPreventAssertInCompareTreePosition = true; - int32_t rVal = nsLayoutUtils::CompareTreePosition(aControl1, aControl2, aForm); + int32_t rVal = nsLayoutUtils::CompareTreePosition(aElement1, aElement2, aForm); nsLayoutUtils::gPreventAssertInCompareTreePosition = false; return rVal; #else // DEBUG - return nsLayoutUtils::CompareTreePosition(aControl1, aControl2, aForm); + return nsLayoutUtils::CompareTreePosition(aElement1, aElement2, aForm); #endif // DEBUG } - + #ifdef DEBUG /** * Checks that all form elements are in document order. Asserts if any pair of @@ -1106,6 +1165,57 @@ nsHTMLFormElement::PostPasswordEvent() event->PostDOMEvent(); } +// This function return true if the element, once appended, is the last one in +// the array. +template +static bool +AddElementToList(nsTArray& aList, ElementType* aChild, + nsHTMLFormElement* aForm) +{ + NS_ASSERTION(aList.IndexOf(aChild) == aList.NoIndex, + "aChild already in aList"); + + 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 = 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 { + int32_t low = 0, mid, high; + high = count - 1; + + while (low <= high) { + mid = (low + high) / 2; + + element = aList[mid]; + position = CompareFormControlPosition(aChild, element, aForm); + if (position >= 0) + low = mid + 1; + else + high = mid - 1; + } + + // WEAK - don't addref + aList.InsertElementAt(low, aChild); + } + + return lastElement; +} + nsresult nsHTMLFormElement::AddElement(nsGenericHTMLFormElement* aChild, bool aUpdateValidity, bool aNotify) @@ -1121,47 +1231,8 @@ nsHTMLFormElement::AddElement(nsGenericHTMLFormElement* aChild, bool childInElements = ShouldBeInElements(aChild); nsTArray& controlList = childInElements ? mControls->mElements : mControls->mNotInElements; - - NS_ASSERTION(controlList.IndexOf(aChild) == controlList.NoIndex, - "Form control already in form"); - uint32_t count = controlList.Length(); - nsGenericHTMLFormElement* element; - - // Optimize most common case where we insert at the end. - bool lastElement = false; - int32_t position = -1; - if (count > 0) { - element = controlList[count - 1]; - position = CompareFormControlPosition(aChild, element, this); - } - - // 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 - controlList.AppendElement(aChild); - lastElement = true; - } - else { - int32_t low = 0, mid, high; - high = count - 1; - - while (low <= high) { - mid = (low + high) / 2; - - element = controlList[mid]; - position = CompareFormControlPosition(aChild, element, this); - if (position >= 0) - low = mid + 1; - else - high = mid - 1; - } - - // WEAK - don't addref - controlList.InsertElementAt(low, aChild); - } + bool lastElement = AddElementToList(controlList, aChild, this); #ifdef DEBUG AssertDocumentOrder(controlList, this); @@ -1359,6 +1430,55 @@ nsHTMLFormElement::HandleDefaultSubmitRemoval() } } +static nsresult +RemoveElementFromTableInternal( + nsInterfaceHashtable& aTable, + nsIContent* aChild, const nsAString& aName) +{ + nsCOMPtr supports; + + if (!aTable.Get(aName, getter_AddRefs(supports))) + return NS_OK; + + nsCOMPtr content(do_QueryInterface(supports)); + + if (content) { + // Single element in the hash, just remove it if it's the one + // we're trying to remove... + if (content == aChild) { + aTable.Remove(aName); + } + + return NS_OK; + } + + nsCOMPtr nodeList(do_QueryInterface(supports)); + NS_ENSURE_TRUE(nodeList, NS_ERROR_FAILURE); + + // Upcast, uggly, but it works! + nsBaseContentList *list = static_cast(nodeList.get()); + + list->RemoveElement(aChild); + + uint32_t length = 0; + list->GetLength(&length); + + if (!length) { + // If the list is empty we remove if from our hash, this shouldn't + // happen tho + aTable.Remove(aName); + } else if (length == 1) { + // Only one element left, replace the list in the hash with the + // single element. + nsIContent* node = list->Item(0); + if (node) { + aTable.Put(aName, node); + } + } + + return NS_OK; +} + nsresult nsHTMLFormElement::RemoveElementFromTable(nsGenericHTMLFormElement* aElement, const nsAString& aName) @@ -1377,13 +1497,13 @@ nsHTMLFormElement::FindNamedItem(const nsAString& aName, return result.forget(); } - nsCOMPtr htmlDoc = do_QueryInterface(GetCurrentDoc()); - if (!htmlDoc) { + result = mImageNameLookupTable.GetWeak(aName); + if (result) { *aCache = nullptr; - return nullptr; + return result.forget(); } - return htmlDoc->ResolveName(aName, this, aCache); + return nullptr; } already_AddRefed @@ -1910,7 +2030,7 @@ nsHTMLFormElement::OnSecurityChange(nsIWebProgress* aWebProgress, NS_NOTREACHED("notification excluded in AddProgressListener(...)"); return NS_OK; } - + NS_IMETHODIMP_(int32_t) nsHTMLFormElement::IndexOfControl(nsIFormControl* aControl) { @@ -2141,6 +2261,16 @@ nsHTMLFormElement::IntrinsicState() const return state; } +void +nsHTMLFormElement::Clear() +{ + for (int32_t i = mImageElements.Length() - 1; i >= 0; i--) { + mImageElements[i]->ClearForm(false); + } + mImageElements.Clear(); + mImageNameLookupTable.Clear(); +} + //---------------------------------------------------------------------- // nsFormControlList implementation, this could go away if there were // a lightweight collection implementation somewhere @@ -2229,7 +2359,7 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE(nsFormControlList) // nsIDOMHTMLCollection interface -NS_IMETHODIMP +NS_IMETHODIMP nsFormControlList::GetLength(uint32_t* aLength) { FlushPendingNotifications(); @@ -2298,20 +2428,17 @@ nsFormControlList::NamedItemInternal(const nsAString& aName, return mNameLookupTable.GetWeak(aName); } -nsresult -nsFormControlList::AddElementToTable(nsGenericHTMLFormElement* aChild, - const nsAString& aName) +static nsresult +AddElementToTableInternal( + nsInterfaceHashtable& aTable, + nsIContent* aChild, const nsAString& aName, nsHTMLFormElement* aForm) { - if (!ShouldBeInElements(aChild)) { - return NS_OK; - } - nsCOMPtr supports; - mNameLookupTable.Get(aName, getter_AddRefs(supports)); + aTable.Get(aName, getter_AddRefs(supports)); if (!supports) { - // No entry found, add the form control - mNameLookupTable.Put(aName, NS_ISUPPORTS_CAST(nsIContent*, aChild)); + // No entry found, add the element + aTable.Put(aName, aChild); } else { // Found something in the hash, check its type nsCOMPtr content = do_QueryInterface(supports); @@ -2327,7 +2454,7 @@ nsFormControlList::AddElementToTable(nsGenericHTMLFormElement* aChild, // Found an element, create a list, add the element to the list and put // the list in the hash - nsSimpleContentList *list = new nsSimpleContentList(mForm); + nsSimpleContentList *list = new nsSimpleContentList(aForm); // If an element has a @form, we can assume it *might* be able to not have // a parent and still be in the form. @@ -2337,14 +2464,14 @@ nsFormControlList::AddElementToTable(nsGenericHTMLFormElement* aChild, // Determine the ordering between the new and old element. bool newFirst = nsContentUtils::PositionIsBefore(aChild, content); - list->AppendElement(newFirst ? aChild : content); - list->AppendElement(newFirst ? content : aChild); + list->AppendElement(newFirst ? aChild : content.get()); + list->AppendElement(newFirst ? content.get() : aChild); nsCOMPtr listSupports = do_QueryObject(list); // Replace the element with the list. - mNameLookupTable.Put(aName, listSupports); + aTable.Put(aName, listSupports); } else { // There's already a list in the hash, add the child to the list nsCOMPtr nodeList = do_QueryInterface(supports); @@ -2395,6 +2522,17 @@ nsFormControlList::AddElementToTable(nsGenericHTMLFormElement* aChild, return NS_OK; } +nsresult +nsFormControlList::AddElementToTable(nsGenericHTMLFormElement* aChild, + const nsAString& aName) +{ + if (!ShouldBeInElements(aChild)) { + return NS_OK; + } + + return AddElementToTableInternal(mNameLookupTable, aChild, aName, mForm); +} + nsresult nsFormControlList::IndexOfControl(nsIFormControl* aControl, int32_t* aIndex) @@ -2416,48 +2554,7 @@ nsFormControlList::RemoveElementFromTable(nsGenericHTMLFormElement* aChild, return NS_OK; } - nsCOMPtr supports; - - if (!mNameLookupTable.Get(aName, getter_AddRefs(supports))) - return NS_OK; - - nsCOMPtr fctrl(do_QueryInterface(supports)); - - if (fctrl) { - // Single element in the hash, just remove it if it's the one - // we're trying to remove... - if (fctrl == aChild) { - mNameLookupTable.Remove(aName); - } - - return NS_OK; - } - - nsCOMPtr nodeList(do_QueryInterface(supports)); - NS_ENSURE_TRUE(nodeList, NS_ERROR_FAILURE); - - // Upcast, uggly, but it works! - nsBaseContentList *list = static_cast(nodeList.get()); - - list->RemoveElement(aChild); - - uint32_t length = 0; - list->GetLength(&length); - - if (!length) { - // If the list is empty we remove if from our hash, this shouldn't - // happen tho - mNameLookupTable.Remove(aName); - } else if (length == 1) { - // Only one element left, replace the list in the hash with the - // single element. - nsIContent* node = list->Item(0); - if (node) { - mNameLookupTable.Put(aName, node); - } - } - - return NS_OK; + return RemoveElementFromTableInternal(mNameLookupTable, aChild, aName); } nsresult @@ -2581,3 +2678,35 @@ nsFormControlList::GetSupportedNames(nsTArray& aNames) // this enumeration. mNameLookupTable.EnumerateRead(CollectNames, &aNames); } + +nsresult +nsHTMLFormElement::AddImageElement(HTMLImageElement* aChild) +{ + AddElementToList(mImageElements, aChild, this); + return NS_OK; +} + +nsresult +nsHTMLFormElement::AddImageElementToTable(HTMLImageElement* aChild, + const nsAString& aName) +{ + return AddElementToTableInternal(mImageNameLookupTable, aChild, aName, this); +} + +nsresult +nsHTMLFormElement::RemoveImageElement(HTMLImageElement* aChild) +{ + uint32_t index = mImageElements.IndexOf(aChild); + NS_ENSURE_STATE(index != mImageElements.NoIndex); + + mImageElements.RemoveElementAt(index); + return NS_OK; +} + +nsresult +nsHTMLFormElement::RemoveImageElementFromTable(HTMLImageElement* aElement, + const nsAString& aName) +{ + return RemoveElementFromTableInternal(mImageNameLookupTable, aElement, aName); +} + diff --git a/content/html/content/src/nsHTMLFormElement.h b/content/html/content/src/nsHTMLFormElement.h index 58f3ea271e8..00ab0bbe9e7 100644 --- a/content/html/content/src/nsHTMLFormElement.h +++ b/content/html/content/src/nsHTMLFormElement.h @@ -32,6 +32,12 @@ class nsFormControlList; class nsIMutableArray; class nsIURI; +namespace mozilla { +namespace dom { +class HTMLImageElement; +} +} + class nsHTMLFormElement : public nsGenericHTMLElement, public nsIDOMHTMLFormElement, public nsIWebProgressListener, @@ -121,8 +127,8 @@ public: virtual nsresult Clone(nsINodeInfo *aNodeInfo, nsINode **aResult) const MOZ_OVERRIDE; - NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED_NO_UNLINK(nsHTMLFormElement, - nsGenericHTMLElement) + NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(nsHTMLFormElement, + nsGenericHTMLElement) /** * Remove an element from this form's list of elements @@ -158,7 +164,7 @@ public: nsresult AddElement(nsGenericHTMLFormElement* aElement, bool aUpdateValidity, bool aNotify); - /** + /** * Add an element to the lookup table maintained by the form. * * We can't fold this method into AddElement() because when @@ -168,6 +174,46 @@ public: */ nsresult AddElementToTable(nsGenericHTMLFormElement* aChild, const nsAString& aName); + + /** + * Remove an image element from this form's list of image elements + * + * @param aElement the image element to remove + * @return NS_OK if the element was successfully removed. + */ + nsresult RemoveImageElement(mozilla::dom::HTMLImageElement* aElement); + + /** + * Remove an image element from the lookup table maintained by the form. + * We can't fold this method into RemoveImageElement() because when + * RemoveImageElement() is called it doesn't know if the element is + * removed because the id attribute has changed, or because the + * name attribute has changed. + * + * @param aElement the image element to remove + * @param aName the name or id of the element to remove + * @return NS_OK if the element was successfully removed. + */ + nsresult RemoveImageElementFromTable(mozilla::dom::HTMLImageElement* aElement, + const nsAString& aName); + /** + * Add an image element to the end of this form's list of image elements + * + * @param aElement the element to add + * @return NS_OK if the element was successfully added + */ + nsresult AddImageElement(mozilla::dom::HTMLImageElement* aElement); + + /** + * Add an image element to the lookup table maintained by the form. + * + * We can't fold this method into AddImageElement() because when + * AddImageElement() is called, the image attributes can change. + * The name or id attributes of the image are used as a key into the table. + */ + nsresult AddImageElementToTable(mozilla::dom::HTMLImageElement* aChild, + const nsAString& aName); + /** * Return whether there is one and only one input text control. * @@ -363,6 +409,9 @@ protected: */ bool CheckFormValidity(nsIMutableArray* aInvalidElements) const; + // Clear the mImageNameLookupTable and mImageElements. + void Clear(); + public: /** * Flush a possible pending submission. If there was a scripted submission @@ -417,6 +466,20 @@ protected: /** The first submit element in mNotInElements -- WEAK */ nsGenericHTMLFormElement* mFirstSubmitNotInElements; + // This array holds on to all HTMLImageElement(s). + // This is needed to properly clean up the bi-directional references + // (both weak and strong) between the form and its HTMLImageElements. + + nsTArray mImageElements; // Holds WEAK references + + // A map from an ID or NAME attribute to the HTMLImageElement(s), this + // hash holds strong references either to the named HTMLImageElement, or + // to a list of named HTMLImageElement(s), in the case where this hash + // holds on to a list of named HTMLImageElement(s) the list has weak + // references to the HTMLImageElement. + + nsInterfaceHashtable mImageNameLookupTable; + /** * Number of invalid and candidate for constraint validation elements in the * form the last time UpdateValidity has been called. diff --git a/content/html/content/test/Makefile.in b/content/html/content/test/Makefile.in index aab783c5825..6e8be161634 100644 --- a/content/html/content/test/Makefile.in +++ b/content/html/content/test/Makefile.in @@ -357,6 +357,7 @@ MOCHITEST_FILES = \ wakelock.ogg \ wakelock.ogv \ test_bug869040.html \ + test_bug870787.html \ allowMedia.sjs \ $(NULL) diff --git a/content/html/content/test/test_bug870787.html b/content/html/content/test/test_bug870787.html new file mode 100644 index 00000000000..92a87128473 --- /dev/null +++ b/content/html/content/test/test_bug870787.html @@ -0,0 +1,84 @@ + + + + + Test for Bug 870787 + + + + + +Mozilla Bug 870787 + +

+ +
+ + + +
+ +
+ + + + +
+
+ + + +
+ + + +
+ +
+ +
+
+
+ + diff --git a/content/html/document/src/nsHTMLDocument.cpp b/content/html/document/src/nsHTMLDocument.cpp index 2911cd85ab5..24947900ed5 100644 --- a/content/html/document/src/nsHTMLDocument.cpp +++ b/content/html/document/src/nsHTMLDocument.cpp @@ -2295,39 +2295,6 @@ nsHTMLDocument::ResolveName(const nsAString& aName, nsWrapperCache **aCache) return nullptr; } -already_AddRefed -nsHTMLDocument::ResolveName(const nsAString& aName, - nsIContent *aForm, - nsWrapperCache **aCache) -{ - nsISupports* result = ResolveName(aName, aCache); - if (!result) { - return nullptr; - } - - nsCOMPtr node = do_QueryInterface(result); - if (!node) { - // We create a nsFormContentList which will filter out the elements in the - // list that don't belong to aForm. - nsRefPtr list = - new nsFormContentList(aForm, *static_cast(result)); - if (list->Length() > 1) { - *aCache = list; - return list.forget(); - } - - // After the nsFormContentList is done filtering there's either nothing or - // one element in the list. Return that element, or null if there's no - // element in the list. - node = list->Item(0); - } else if (!nsContentUtils::BelongsInForm(aForm, node)) { - node = nullptr; - } - - *aCache = node; - return node.forget(); -} - JSObject* nsHTMLDocument::NamedGetter(JSContext* cx, const nsAString& aName, bool& aFound, ErrorResult& rv) diff --git a/content/html/document/src/nsHTMLDocument.h b/content/html/document/src/nsHTMLDocument.h index f88b6a5e8e4..c9bf58f38a6 100644 --- a/content/html/document/src/nsHTMLDocument.h +++ b/content/html/document/src/nsHTMLDocument.h @@ -115,9 +115,6 @@ public: JSObject* GetAll(JSContext* aCx, mozilla::ErrorResult& aRv); nsISupports* ResolveName(const nsAString& aName, nsWrapperCache **aCache); - virtual already_AddRefed ResolveName(const nsAString& aName, - nsIContent *aForm, - nsWrapperCache **aCache) MOZ_OVERRIDE; virtual void AddedForm() MOZ_OVERRIDE; virtual void RemovedForm() MOZ_OVERRIDE; diff --git a/content/html/document/src/nsIHTMLDocument.h b/content/html/document/src/nsIHTMLDocument.h index 34dc515359a..db8618a3b10 100644 --- a/content/html/document/src/nsIHTMLDocument.h +++ b/content/html/document/src/nsIHTMLDocument.h @@ -33,10 +33,6 @@ public: */ virtual void SetCompatibilityMode(nsCompatibility aMode) = 0; - virtual already_AddRefed ResolveName(const nsAString& aName, - nsIContent *aForm, - nsWrapperCache **aCache) = 0; - /** * Called when form->BindToTree() is called so that document knows * immediately when a form is added