From 162b40b52e93ecc84d724cd9af28368d7f1d7c08 Mon Sep 17 00:00:00 2001 From: Alexander Surkov Date: Wed, 17 Nov 2010 21:55:44 -0500 Subject: [PATCH] Bug 573469 - part2, cache IDs pointed by relation attributes, r=marcoz, davidb, sr=neil, a=blockingFinal+ --- accessible/src/base/AccIterator.cpp | 46 +++++- accessible/src/base/AccIterator.h | 38 +++++ accessible/src/base/nsAccessible.cpp | 75 ++++++---- accessible/src/base/nsAccessible.h | 3 - accessible/src/base/nsDocAccessible.cpp | 115 ++++++++++++++- accessible/src/base/nsDocAccessible.h | 63 ++++++-- accessible/tests/mochitest/relations/Makefile.in | 1 + .../tests/mochitest/relations/test_general.html | 3 + .../tests/mochitest/relations/test_general.xul | 3 + .../tests/mochitest/relations/test_update.html | 162 +++++++++++++++++++++ 10 files changed, 467 insertions(+), 42 deletions(-) create mode 100644 accessible/tests/mochitest/relations/test_update.html diff --git a/accessible/src/base/AccIterator.cpp b/accessible/src/base/AccIterator.cpp index 1629f20d9a..23ace181c8 100644 --- a/accessible/src/base/AccIterator.cpp +++ b/accessible/src/base/AccIterator.cpp @@ -37,10 +37,12 @@ #include "AccIterator.h" +#include "nsAccessibilityService.h" #include "nsAccessible.h" //////////////////////////////////////////////////////////////////////////////// -// nsAccIterator +// AccIterator +//////////////////////////////////////////////////////////////////////////////// AccIterator::AccIterator(nsAccessible *aAccessible, filters::FilterFuncPtr aFilterFunc, @@ -93,3 +95,45 @@ AccIterator::IteratorState::IteratorState(nsAccessible *aParent, mParent(aParent), mIndex(0), mParentState(mParentState) { } + + +//////////////////////////////////////////////////////////////////////////////// +// RelatedAccIterator +//////////////////////////////////////////////////////////////////////////////// + +RelatedAccIterator:: + RelatedAccIterator(nsDocAccessible* aDocument, nsIContent* aDependentContent, + nsIAtom* aRelAttr) : + mRelAttr(aRelAttr), mProviders(nsnull), mBindingParent(nsnull), mIndex(0) +{ + mBindingParent = aDependentContent->GetBindingParent(); + nsIAtom* IDAttr = mBindingParent ? + nsAccessibilityAtoms::anonid : aDependentContent->GetIDAttributeName(); + + nsAutoString id; + if (aDependentContent->GetAttr(kNameSpaceID_None, IDAttr, id)) + mProviders = aDocument->mDependentIDsHash.Get(id); +} + +nsAccessible* +RelatedAccIterator::Next() +{ + if (!mProviders) + return nsnull; + + while (mIndex < mProviders->Length()) { + nsDocAccessible::AttrRelProvider* provider = (*mProviders)[mIndex++]; + + // Return related accessible for the given attribute and if the provider + // content is in the same binding in the case of XBL usage. + if (provider->mRelAttr == mRelAttr && + (!mBindingParent || + mBindingParent == provider->mContent->GetBindingParent())) { + nsAccessible* related = GetAccService()->GetAccessible(provider->mContent); + if (related) + return related; + } + } + + return nsnull; +} diff --git a/accessible/src/base/AccIterator.h b/accessible/src/base/AccIterator.h index 6c07b62f7e..a78aefd7da 100644 --- a/accessible/src/base/AccIterator.h +++ b/accessible/src/base/AccIterator.h @@ -40,6 +40,7 @@ #include "filters.h" #include "nscore.h" +#include "nsDocAccessible.h" /** * Allows to iterate through accessible children or subtree complying with @@ -93,4 +94,41 @@ private: IteratorState *mState; }; + +/** + * Allows to traverse through related accessibles that are pointing to the given + * dependent accessible by relation attribute. + */ +class RelatedAccIterator +{ +public: + /** + * Constructor. + * + * @param aDocument [in] the document accessible the related + * & accessibles belong to. + * @param aDependentContent [in] the content of dependent accessible that + * relations were requested for + * @param aRelAttr [in] relation attribute that relations are + * pointed by + */ + RelatedAccIterator(nsDocAccessible* aDocument, nsIContent* aDependentContent, + nsIAtom* aRelAttr); + + /** + * Return next related accessible for the given dependent accessible. + */ + nsAccessible* Next(); + +private: + RelatedAccIterator(); + RelatedAccIterator(const RelatedAccIterator&); + RelatedAccIterator& operator = (const RelatedAccIterator&); + + nsIAtom* mRelAttr; + nsDocAccessible::AttrRelProviderArray* mProviders; + nsIContent* mBindingParent; + PRUint32 mIndex; +}; + #endif diff --git a/accessible/src/base/nsAccessible.cpp b/accessible/src/base/nsAccessible.cpp index e0e0753bdb..c2682bd7cb 100644 --- a/accessible/src/base/nsAccessible.cpp +++ b/accessible/src/base/nsAccessible.cpp @@ -2037,25 +2037,28 @@ nsAccessible::GetRelationByType(PRUint32 aRelationType, // Relationships are defined on the same content node that the role would be // defined on. - nsresult rv; + nsresult rv = NS_OK_NO_RELATION_TARGET; switch (aRelationType) { case nsIAccessibleRelation::RELATION_LABEL_FOR: { + RelatedAccIterator iter(GetDocAccessible(), mContent, + nsAccessibilityAtoms::aria_labelledby); + + nsAccessible* related = nsnull; + while ((related = iter.Next())) { + rv = nsRelUtils::AddTarget(aRelationType, aRelation, related); + NS_ENSURE_SUCCESS(rv, rv); + } + if (mContent->Tag() == nsAccessibilityAtoms::label) { nsIAtom *IDAttr = mContent->IsHTML() ? nsAccessibilityAtoms::_for : nsAccessibilityAtoms::control; rv = nsRelUtils:: AddTargetFromIDRefAttr(aRelationType, aRelation, mContent, IDAttr); NS_ENSURE_SUCCESS(rv, rv); - - if (rv != NS_OK_NO_RELATION_TARGET) - return NS_OK; // XXX bug 381599, avoid performance problems } - - return nsRelUtils:: - AddTargetFromNeighbour(aRelationType, aRelation, mContent, - nsAccessibilityAtoms::aria_labelledby); + return rv; } case nsIAccessibleRelation::RELATION_LABELLED_BY: @@ -2091,13 +2094,14 @@ nsAccessible::GetRelationByType(PRUint32 aRelationType, case nsIAccessibleRelation::RELATION_DESCRIPTION_FOR: { - rv = nsRelUtils:: - AddTargetFromNeighbour(aRelationType, aRelation, mContent, - nsAccessibilityAtoms::aria_describedby); - NS_ENSURE_SUCCESS(rv, rv); + RelatedAccIterator iter(GetDocAccessible(), mContent, + nsAccessibilityAtoms::aria_describedby); - if (rv != NS_OK_NO_RELATION_TARGET) - return NS_OK; // XXX bug 381599, avoid performance problems + nsAccessible* related = nsnull; + while ((related = iter.Next())) { + rv = nsRelUtils::AddTarget(aRelationType, aRelation, related); + NS_ENSURE_SUCCESS(rv, rv); + } if (mContent->Tag() == nsAccessibilityAtoms::description && mContent->IsXUL()) { @@ -2109,18 +2113,23 @@ nsAccessible::GetRelationByType(PRUint32 aRelationType, nsAccessibilityAtoms::control); } - return NS_OK; + return rv; } case nsIAccessibleRelation::RELATION_NODE_CHILD_OF: { - rv = nsRelUtils:: - AddTargetFromNeighbour(aRelationType, aRelation, mContent, - nsAccessibilityAtoms::aria_owns); - NS_ENSURE_SUCCESS(rv, rv); + RelatedAccIterator iter(GetDocAccessible(), mContent, + nsAccessibilityAtoms::aria_owns); + nsAccessible* related = nsnull; + while ((related = iter.Next())) { + rv = nsRelUtils::AddTarget(aRelationType, aRelation, related); + NS_ENSURE_SUCCESS(rv, rv); + } + + // Got relation from aria-owns, don't calculate it from native markup. if (rv != NS_OK_NO_RELATION_TARGET) - return NS_OK; // XXX bug 381599, avoid performance problems + return NS_OK; // This is an ARIA tree or treegrid that doesn't use owns, so we need to // get the parent the hard way. @@ -2153,14 +2162,20 @@ nsAccessible::GetRelationByType(PRUint32 aRelationType, } } - return NS_OK; + return rv; } case nsIAccessibleRelation::RELATION_CONTROLLED_BY: { - return nsRelUtils:: - AddTargetFromNeighbour(aRelationType, aRelation, mContent, - nsAccessibilityAtoms::aria_controls); + RelatedAccIterator iter(GetDocAccessible(), mContent, + nsAccessibilityAtoms::aria_controls); + + nsAccessible* related = nsnull; + while ((related = iter.Next())) { + rv = nsRelUtils::AddTarget(aRelationType, aRelation, related); + NS_ENSURE_SUCCESS(rv, rv); + } + return rv; } case nsIAccessibleRelation::RELATION_CONTROLLER_FOR: @@ -2188,9 +2203,15 @@ nsAccessible::GetRelationByType(PRUint32 aRelationType, case nsIAccessibleRelation::RELATION_FLOWS_FROM: { - return nsRelUtils:: - AddTargetFromNeighbour(aRelationType, aRelation, mContent, - nsAccessibilityAtoms::aria_flowto); + RelatedAccIterator iter(GetDocAccessible(), mContent, + nsAccessibilityAtoms::aria_flowto); + + nsAccessible* related = nsnull; + while ((related = iter.Next())) { + rv = nsRelUtils::AddTarget(aRelationType, aRelation, related); + NS_ENSURE_SUCCESS(rv, rv); + } + return rv; } case nsIAccessibleRelation::RELATION_DEFAULT_BUTTON: diff --git a/accessible/src/base/nsAccessible.h b/accessible/src/base/nsAccessible.h index a01fbe3b7e..bd4b0b0bb2 100644 --- a/accessible/src/base/nsAccessible.h +++ b/accessible/src/base/nsAccessible.h @@ -51,7 +51,6 @@ #include "nsStringGlue.h" #include "nsTArray.h" #include "nsRefPtrHashtable.h" -#include "nsDataHashtable.h" class AccGroupInfo; class EmbeddedObjCollector; @@ -67,8 +66,6 @@ class nsIView; typedef nsRefPtrHashtable nsAccessibleHashtable; -typedef nsDataHashtable, nsAccessible*> - NodeToAccessibleMap; // see nsAccessible::GetAttrValue #define NS_OK_NO_ARIA_VALUE \ diff --git a/accessible/src/base/nsDocAccessible.cpp b/accessible/src/base/nsDocAccessible.cpp index 3016e0e9d3..1a9d582ae5 100644 --- a/accessible/src/base/nsDocAccessible.cpp +++ b/accessible/src/base/nsDocAccessible.cpp @@ -85,6 +85,16 @@ namespace dom = mozilla::dom; PRUint32 nsDocAccessible::gLastFocusedAccessiblesState = 0; +static nsIAtom** kRelationAttrs[] = +{ + &nsAccessibilityAtoms::aria_labelledby, + &nsAccessibilityAtoms::aria_describedby, + &nsAccessibilityAtoms::aria_owns, + &nsAccessibilityAtoms::aria_controls, + &nsAccessibilityAtoms::aria_flowto +}; + +static const PRUint32 kRelationAttrsLen = NS_ARRAY_LENGTH(kRelationAttrs); //////////////////////////////////////////////////////////////////////////////// // Constructor/desctructor @@ -95,6 +105,7 @@ nsDocAccessible:: nsHyperTextAccessibleWrap(aRootContent, aShell), mDocument(aDocument), mScrollPositionChangedTicks(0), mIsLoaded(PR_FALSE) { + mDependentIDsHash.Init(); // XXX aaronl should we use an algorithm for the initial cache size? mAccessibleCache.Init(kDefaultCacheSize); mNodeToAccessibleMap.Init(kDefaultCacheSize); @@ -134,6 +145,7 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(nsDocAccessible, nsAccessible) NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mEventQueue) NS_IMPL_CYCLE_COLLECTION_UNLINK_NSTARRAY(mChildDocuments) + tmp->mDependentIDsHash.Clear(); tmp->mNodeToAccessibleMap.Clear(); ClearCache(tmp->mAccessibleCache); NS_IMPL_CYCLE_COLLECTION_UNLINK_END @@ -664,6 +676,7 @@ nsDocAccessible::Shutdown() mWeakShell = nsnull; // Avoid reentrancy + mDependentIDsHash.Clear(); mNodeToAccessibleMap.Clear(); ClearCache(mAccessibleCache); @@ -929,10 +942,19 @@ nsDocAccessible::AttributeWillChange(nsIDocument *aDocument, PRInt32 aNameSpaceID, nsIAtom* aAttribute, PRInt32 aModType) { - // XXX TODO: bugs 381599 467143 472142 472143 + // XXX TODO: bugs 381599 (partially fixed by 573469), 467143, 472142, 472143. // Here we will want to cache whatever state we are potentially interested in, // such as the existence of aria-pressed for button (so we know if we need to // newly expose it as a toggle button) etc. + + // Update dependent IDs cache. + if (aModType == nsIDOMMutationEvent::MODIFICATION || + aModType == nsIDOMMutationEvent::REMOVAL) { + nsAccessible* accessible = + GetAccService()->GetAccessibleInWeakShell(aElement, mWeakShell); + if (accessible) + RemoveDependentIDsFor(accessible, aAttribute); + } } void @@ -943,6 +965,16 @@ nsDocAccessible::AttributeChanged(nsIDocument *aDocument, { AttributeChangedImpl(aElement, aNameSpaceID, aAttribute); + // Update dependent IDs cache. + if (aModType == nsIDOMMutationEvent::MODIFICATION || + aModType == nsIDOMMutationEvent::ADDITION) { + nsAccessible* accessible = + GetAccService()->GetAccessibleInWeakShell(aElement, mWeakShell); + + if (accessible) + AddDependentIDsFor(accessible, aAttribute); + } + // If it was the focused node, cache the new state if (aElement == gLastFocusedNode) { nsAccessible *focusedAccessible = GetAccService()->GetAccessible(aElement); @@ -1362,6 +1394,7 @@ nsDocAccessible::BindToDocument(nsAccessible* aAccessible, } aAccessible->SetRoleMapEntry(aRoleMapEntry); + AddDependentIDsFor(aAccessible); return true; } @@ -1373,6 +1406,8 @@ nsDocAccessible::UnbindFromDocument(nsAccessible* aAccessible) mNodeToAccessibleMap.Get(aAccessible->GetNode()) == aAccessible) mNodeToAccessibleMap.Remove(aAccessible->GetNode()); + RemoveDependentIDsFor(aAccessible); + #ifdef DEBUG NS_ASSERTION(mAccessibleCache.GetWeak(aAccessible->UniqueID()), "Unbinding the unbound accessible!"); @@ -1558,6 +1593,84 @@ nsDocAccessible::RecreateAccessible(nsINode* aNode) // Protected members void +nsDocAccessible::AddDependentIDsFor(nsAccessible* aRelProvider, + nsIAtom* aRelAttr) +{ + for (PRUint32 idx = 0; idx < kRelationAttrsLen; idx++) { + nsIAtom* relAttr = *kRelationAttrs[idx]; + if (aRelAttr && aRelAttr != relAttr) + continue; + + IDRefsIterator iter(aRelProvider->GetContent(), relAttr); + while (true) { + const nsDependentSubstring id = iter.NextID(); + if (id.IsEmpty()) + break; + + AttrRelProviderArray* providers = mDependentIDsHash.Get(id); + if (!providers) { + providers = new AttrRelProviderArray(); + if (providers) { + if (!mDependentIDsHash.Put(id, providers)) { + delete providers; + providers = nsnull; + } + } + } + + if (providers) { + AttrRelProvider* provider = + new AttrRelProvider(relAttr, aRelProvider->GetContent()); + if (provider) + providers->AppendElement(provider); + } + } + + // If the relation attribute is given then we don't have anything else to + // check. + if (aRelAttr) + break; + } +} + +void +nsDocAccessible::RemoveDependentIDsFor(nsAccessible* aRelProvider, + nsIAtom* aRelAttr) +{ + for (PRUint32 idx = 0; idx < kRelationAttrsLen; idx++) { + nsIAtom* relAttr = *kRelationAttrs[idx]; + if (aRelAttr && aRelAttr != *kRelationAttrs[idx]) + continue; + + IDRefsIterator iter(aRelProvider->GetContent(), relAttr); + while (true) { + const nsDependentSubstring id = iter.NextID(); + if (id.IsEmpty()) + break; + + AttrRelProviderArray* providers = mDependentIDsHash.Get(id); + if (providers) { + for (PRUint32 jdx = 0; jdx < providers->Length(); ) { + AttrRelProvider* provider = (*providers)[jdx]; + if (provider->mRelAttr == relAttr && + provider->mContent == aRelProvider->GetContent()) + providers->RemoveElement(provider); + else + jdx++; + } + if (providers->Length() == 0) + mDependentIDsHash.Remove(id); + } + } + + // If the relation attribute is given then we don't have anything else to + // check. + if (aRelAttr) + break; + } +} + +void nsDocAccessible::FireValueChangeForTextFields(nsAccessible *aAccessible) { if (aAccessible->Role() != nsIAccessibleRole::ROLE_ENTRY) diff --git a/accessible/src/base/nsDocAccessible.h b/accessible/src/base/nsDocAccessible.h index 7ea913d368..3ce0303e13 100644 --- a/accessible/src/base/nsDocAccessible.h +++ b/accessible/src/base/nsDocAccessible.h @@ -44,6 +44,8 @@ #include "nsHyperTextAccessibleWrap.h" #include "nsEventShell.h" +#include "nsClassHashtable.h" +#include "nsDataHashtable.h" #include "nsIDocument.h" #include "nsIDocumentObserver.h" #include "nsIEditor.h" @@ -267,6 +269,28 @@ protected: mChildDocuments.RemoveElement(aChildDocument); } + /** + * Add dependent IDs pointed by accessible element by relation attribute to + * cache. If the relation attribute is missed then all relation attributes + * are checked. + * + * @param aRelProvider [in] accessible that element has relation attribute + * @param aRelAttr [in, optional] relation attribute + */ + void AddDependentIDsFor(nsAccessible* aRelProvider, + nsIAtom* aRelAttr = nsnull); + + /** + * Remove dependent IDs pointed by accessible element by relation attribute + * from cache. If the relation attribute is absent then all relation + * attributes are checked. + * + * @param aRelProvider [in] accessible that element has relation attribute + * @param aRelAttr [in, optional] relation attribute + */ + void RemoveDependentIDsFor(nsAccessible* aRelProvider, + nsIAtom* aRelAttr = nsnull); + static void ScrollTimerCallback(nsITimer *aTimer, void *aClosure); /** @@ -300,14 +324,6 @@ protected: PRBool aIsInserted); /** - * Used to define should the event be fired on a delay. - */ - enum EEventFiringType { - eNormalEvent, - eDelayedEvent - }; - - /** * Fire a value change event for the the given accessible if it is a text * field (has a ROLE_ENTRY). */ @@ -347,7 +363,8 @@ protected: * Cache of accessibles within this document accessible. */ nsAccessibleHashtable mAccessibleCache; - NodeToAccessibleMap mNodeToAccessibleMap; + nsDataHashtable, nsAccessible*> + mNodeToAccessibleMap; nsCOMPtr mDocument; nsCOMPtr mScrollWatchTimer; @@ -366,9 +383,35 @@ protected: static nsIAtom *gLastFocusedFrameType; nsTArray > mChildDocuments; + + /** + * A storage class for pairing content with one of its relation attributes. + */ + class AttrRelProvider + { + public: + AttrRelProvider(nsIAtom* aRelAttr, nsIContent* aContent) : + mRelAttr(aRelAttr), mContent(aContent) { } + + nsIAtom* mRelAttr; + nsIContent* mContent; + + private: + AttrRelProvider(); + AttrRelProvider(const AttrRelProvider&); + AttrRelProvider& operator =(const AttrRelProvider&); + }; + + /** + * The cache of IDs pointed by relation attributes. + */ + typedef nsTArray > AttrRelProviderArray; + nsClassHashtable mDependentIDsHash; + + friend class RelatedAccIterator; }; NS_DEFINE_STATIC_IID_ACCESSOR(nsDocAccessible, NS_DOCACCESSIBLE_IMPL_CID) -#endif +#endif diff --git a/accessible/tests/mochitest/relations/Makefile.in b/accessible/tests/mochitest/relations/Makefile.in index 813f7b6a46..de3d64cf20 100644 --- a/accessible/tests/mochitest/relations/Makefile.in +++ b/accessible/tests/mochitest/relations/Makefile.in @@ -50,6 +50,7 @@ _TEST_FILES =\ test_general.xul \ test_tabbrowser.xul \ test_tree.xul \ + test_update.html \ $(NULL) libs:: $(_TEST_FILES) diff --git a/accessible/tests/mochitest/relations/test_general.html b/accessible/tests/mochitest/relations/test_general.html index 5a62fc7c8a..a8fd692c69 100644 --- a/accessible/tests/mochitest/relations/test_general.html +++ b/accessible/tests/mochitest/relations/test_general.html @@ -65,6 +65,9 @@ testRelation(iframeDocAcc, RELATION_NODE_CHILD_OF, iframeAcc); // aria-controls + getAccessible("tab"); + todo(false, + "Getting an accessible tab, otherwise relations for tabpanel aren't cached. Bug 606924 will fix that."); testRelation("tabpanel", RELATION_CONTROLLED_BY, "tab"); testRelation("tab", RELATION_CONTROLLER_FOR, "tabpanel"); diff --git a/accessible/tests/mochitest/relations/test_general.xul b/accessible/tests/mochitest/relations/test_general.xul index 35ab6fd1c8..6a689adc24 100644 --- a/accessible/tests/mochitest/relations/test_general.xul +++ b/accessible/tests/mochitest/relations/test_general.xul @@ -66,6 +66,9 @@ testRelation(iframeDocAcc, RELATION_NODE_CHILD_OF, iframeAcc); // aria-controls + getAccessible("tab"); + todo(false, + "Getting an accessible tab, otherwise relations for tabpanel aren't cached. Bug 606924 will fix that."); testRelation("tabpanel", RELATION_CONTROLLED_BY, "tab"); testRelation("tab", RELATION_CONTROLLER_FOR, "tabpanel"); diff --git a/accessible/tests/mochitest/relations/test_update.html b/accessible/tests/mochitest/relations/test_update.html new file mode 100644 index 0000000000..99dcd6055b --- /dev/null +++ b/accessible/tests/mochitest/relations/test_update.html @@ -0,0 +1,162 @@ + + + + Test updating of accessible relations + + + + + + + + + + + + + + + + + + Mozilla Bug 573469 + +

+ + 
+
+ +
label
+
label2
+ + + + -- 2.11.4.GIT