diff --git a/accessible/base/NotificationController.cpp b/accessible/base/NotificationController.cpp index 77f161048c..ef024cb333 100644 --- a/accessible/base/NotificationController.cpp +++ b/accessible/base/NotificationController.cpp @@ -52,7 +52,16 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(NotificationController) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mHangingChildDocuments) - NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mContentInsertions) + for (auto it = tmp->mContentInsertions.ConstIter(); !it.Done(); it.Next()) { + NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mContentInsertions key"); + cb.NoteXPCOMChild(it.Key()); + nsTArray>* list = it.UserData(); + for (uint32_t i = 0; i < list->Length(); i++) { + NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, + "mContentInsertions value item"); + cb.NoteXPCOMChild(list->ElementAt(i)); + } + } NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mEvents) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mRelocations) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END @@ -103,10 +112,23 @@ NotificationController::ScheduleContentInsertion(Accessible* aContainer, nsIContent* aStartChildNode, nsIContent* aEndChildNode) { - RefPtr insertion = new ContentInsertion(mDocument, - aContainer); - if (insertion && insertion->InitChildList(aStartChildNode, aEndChildNode) && - mContentInsertions.AppendElement(insertion)) { + nsTArray>* list = + mContentInsertions.LookupOrAdd(aContainer); + + bool needsProcessing = false; + nsIContent* node = aStartChildNode; + while (node != aEndChildNode) { + // Notification triggers for content insertion even if no content was + // actually inserted, check if the given content has a frame to discard + // this case early. + if (node->GetPrimaryFrame()) { + if (list->AppendElement(node)) + needsProcessing = true; + } + node = node->GetNextSibling(); + } + + if (needsProcessing) { ScheduleProcessing(); } } @@ -130,7 +152,7 @@ NotificationController::IsUpdatePending() { return mPresShell->IsLayoutFlushObserver() || mObservingState == eRefreshProcessingForUpdate || - mContentInsertions.Length() != 0 || mNotifications.Length() != 0 || + mContentInsertions.Count() != 0 || mNotifications.Length() != 0 || mTextHash.Count() != 0 || !mDocument->HasLoadState(DocAccessible::eTreeConstructed); } @@ -178,7 +200,7 @@ NotificationController::WillRefresh(mozilla::TimeStamp aTime) mDocument->DoInitialUpdate(); - NS_ASSERTION(mContentInsertions.Length() == 0, + NS_ASSERTION(mContentInsertions.Count() == 0, "Pending content insertions while initial accessible tree isn't created!"); } @@ -196,15 +218,13 @@ NotificationController::WillRefresh(mozilla::TimeStamp aTime) // document accessible. // Process only currently queued content inserted notifications. - nsTArray > contentInsertions; - contentInsertions.SwapElements(mContentInsertions); - - uint32_t insertionCount = contentInsertions.Length(); - for (uint32_t idx = 0; idx < insertionCount; idx++) { - contentInsertions[idx]->Process(); - if (!mDocument) + for (auto iter = mContentInsertions.ConstIter(); !iter.Done(); iter.Next()) { + mDocument->ProcessContentInserted(iter.Key(), iter.UserData()); + if (!mDocument) { return; + } } + mContentInsertions.Clear(); // Process rendered text change notifications. for (auto iter = mTextHash.Iter(); !iter.Done(); iter.Next()) { @@ -390,8 +410,10 @@ NotificationController::WillRefresh(mozilla::TimeStamp aTime) childDoc->SetIPCDoc(ipcDoc); nsCOMPtr tabChild = do_GetInterface(mDocument->DocumentNode()->GetDocShell()); - static_cast(tabChild.get())-> - SendPDocAccessibleConstructor(ipcDoc, parentIPCDoc, id); + if (tabChild) { + static_cast(tabChild.get())-> + SendPDocAccessibleConstructor(ipcDoc, parentIPCDoc, id); + } } } @@ -401,7 +423,7 @@ NotificationController::WillRefresh(mozilla::TimeStamp aTime) // Stop further processing if there are no new notifications of any kind or // events and document load is processed. - if (mContentInsertions.IsEmpty() && mNotifications.IsEmpty() && + if (mContentInsertions.Count() == 0 && mNotifications.IsEmpty() && mEvents.IsEmpty() && mTextHash.Count() == 0 && mHangingChildDocuments.IsEmpty() && mDocument->HasLoadState(DocAccessible::eCompletelyLoaded) && @@ -409,53 +431,3 @@ NotificationController::WillRefresh(mozilla::TimeStamp aTime) mObservingState = eNotObservingRefresh; } } - -//////////////////////////////////////////////////////////////////////////////// -// NotificationController: content inserted notification - -NotificationController::ContentInsertion:: - ContentInsertion(DocAccessible* aDocument, Accessible* aContainer) : - mDocument(aDocument), mContainer(aContainer) -{ -} - -bool -NotificationController::ContentInsertion:: - InitChildList(nsIContent* aStartChildNode, nsIContent* aEndChildNode) -{ - bool haveToUpdate = false; - - nsIContent* node = aStartChildNode; - while (node != aEndChildNode) { - // Notification triggers for content insertion even if no content was - // actually inserted, check if the given content has a frame to discard - // this case early. - if (node->GetPrimaryFrame()) { - if (mInsertedContent.AppendElement(node)) - haveToUpdate = true; - } - - node = node->GetNextSibling(); - } - - return haveToUpdate; -} - -NS_IMPL_CYCLE_COLLECTION(NotificationController::ContentInsertion, - mContainer) - -NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(NotificationController::ContentInsertion, - AddRef) -NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(NotificationController::ContentInsertion, - Release) - -void -NotificationController::ContentInsertion::Process() -{ - mDocument->ProcessContentInserted(mContainer, &mInsertedContent); - - mDocument = nullptr; - mContainer = nullptr; - mInsertedContent.Clear(); -} - diff --git a/accessible/base/NotificationController.h b/accessible/base/NotificationController.h index 9ae6f7e6aa..666ddcc4a3 100644 --- a/accessible/base/NotificationController.h +++ b/accessible/base/NotificationController.h @@ -122,8 +122,15 @@ public: */ inline void ScheduleTextUpdate(nsIContent* aTextNode) { - if (mTextHash.PutEntry(aTextNode)) - ScheduleProcessing(); + // Make sure we are not called with a node that is not in the DOM tree or + // not visible. + MOZ_ASSERT(aTextNode->GetParentNode(), "A text node is not in DOM"); + MOZ_ASSERT(aTextNode->GetPrimaryFrame(), "A text node doesn't have a frame"); + MOZ_ASSERT(aTextNode->GetPrimaryFrame()->StyleVisibility()->IsVisible(), + "A text node is not visible"); + + mTextHash.PutEntry(aTextNode); + ScheduleProcessing(); } /** @@ -240,44 +247,10 @@ private: nsTArray > mHangingChildDocuments; /** - * Storage for content inserted notification information. + * Pending accessible tree update notifications for content insertions. */ - class ContentInsertion - { - public: - ContentInsertion(DocAccessible* aDocument, Accessible* aContainer); - - NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(ContentInsertion) - NS_DECL_CYCLE_COLLECTION_NATIVE_CLASS(ContentInsertion) - - bool InitChildList(nsIContent* aStartChildNode, nsIContent* aEndChildNode); - void Process(); - - protected: - virtual ~ContentInsertion() { mDocument = nullptr; } - - private: - ContentInsertion(); - ContentInsertion(const ContentInsertion&); - ContentInsertion& operator = (const ContentInsertion&); - - // The document used to process content insertion, matched to document of - // the notification controller that this notification belongs to, therefore - // it's ok to keep it as weak ref. - DocAccessible* mDocument; - - // The container accessible that content insertion occurs within. - RefPtr mContainer; - - // Array of inserted contents. - nsTArray > mInsertedContent; - }; - - /** - * A pending accessible tree update notifications for content insertions. - * Don't make this an AutoTArray; we use SwapElements() on it. - */ - nsTArray > mContentInsertions; + nsClassHashtable, + nsTArray>> mContentInsertions; template class nsCOMPtrHashKey : public PLDHashEntryHdr @@ -304,7 +277,7 @@ private: }; /** - * A pending accessible tree update notifications for rendered text changes. + * Pending accessible tree update notifications for rendered text changes. */ nsTHashtable > mTextHash; diff --git a/accessible/base/StyleInfo.cpp b/accessible/base/StyleInfo.cpp index c4e2203ffa..51291ed09b 100644 --- a/accessible/base/StyleInfo.cpp +++ b/accessible/base/StyleInfo.cpp @@ -58,6 +58,7 @@ StyleInfo::TextIndent(nsAString& aValue) case eStyleUnit_Percent: { nsIFrame* frame = mElement->GetPrimaryFrame(); + MOZ_ASSERT(frame, "frame must be a valid pointer."); nsIFrame* containerFrame = frame->GetContainingBlock(); nscoord percentageBase = containerFrame->GetContentRect().width; coordVal = NSCoordSaturatingMultiply(percentageBase, @@ -88,6 +89,7 @@ StyleInfo::TextIndent(nsAString& aValue) void StyleInfo::Margin(css::Side aSide, nsAString& aValue) { + MOZ_ASSERT(mElement->GetPrimaryFrame(), " mElement->GetPrimaryFrame() needs to be valid pointer"); aValue.Truncate(); nscoord coordVal = mElement->GetPrimaryFrame()->GetUsedMargin().Side(aSide); diff --git a/accessible/base/nsCoreUtils.cpp b/accessible/base/nsCoreUtils.cpp index 8b12a5196f..6262caecd6 100644 --- a/accessible/base/nsCoreUtils.cpp +++ b/accessible/base/nsCoreUtils.cpp @@ -36,6 +36,7 @@ #include "nsITreeBoxObject.h" #include "nsITreeColumns.h" #include "mozilla/dom/Element.h" +#include "mozilla/dom/HTMLLabelElement.h" using namespace mozilla; @@ -43,6 +44,16 @@ using namespace mozilla; // nsCoreUtils //////////////////////////////////////////////////////////////////////////////// +bool +nsCoreUtils::IsLabelWithControl(nsIContent* aContent) +{ + dom::HTMLLabelElement* label = dom::HTMLLabelElement::FromContent(aContent); + if (label && label->GetControl()) + return true; + + return false; +} + bool nsCoreUtils::HasClickListener(nsIContent *aContent) { diff --git a/accessible/base/nsCoreUtils.h b/accessible/base/nsCoreUtils.h index 78fa99e282..c8660dc46c 100644 --- a/accessible/base/nsCoreUtils.h +++ b/accessible/base/nsCoreUtils.h @@ -29,6 +29,11 @@ class nsIWidget; class nsCoreUtils { public: + /** + * Return true if the given node is a label of a control. + */ + static bool IsLabelWithControl(nsIContent *aContent); + /** * Return true if the given node has registered click, mousedown or mouseup * event listeners. diff --git a/accessible/generic/BaseAccessibles.cpp b/accessible/generic/BaseAccessibles.cpp index 6e79fa8156..04c31d39bb 100644 --- a/accessible/generic/BaseAccessibles.cpp +++ b/accessible/generic/BaseAccessibles.cpp @@ -115,13 +115,14 @@ LinkableAccessible::Value(nsString& aValue) uint8_t LinkableAccessible::ActionCount() { - bool isLink, isOnclick; - ActionWalk(&isLink, &isOnclick); - return (isLink || isOnclick) ? 1 : 0; + bool isLink, isOnclick, isLabelWithControl; + ActionWalk(&isLink, &isOnclick, &isLabelWithControl); + return (isLink || isOnclick || isLabelWithControl) ? 1 : 0; } Accessible* -LinkableAccessible::ActionWalk(bool* aIsLink, bool* aIsOnclick) +LinkableAccessible::ActionWalk(bool* aIsLink, bool* aIsOnclick, + bool* aIsLabelWithControl) { if (aIsOnclick) { *aIsOnclick = false; @@ -129,6 +130,9 @@ LinkableAccessible::ActionWalk(bool* aIsLink, bool* aIsOnclick) if (aIsLink) { *aIsLink = false; } + if (aIsLabelWithControl) { + *aIsLabelWithControl = false; + } if (nsCoreUtils::HasClickListener(mContent)) { if (aIsOnclick) { @@ -155,6 +159,13 @@ LinkableAccessible::ActionWalk(bool* aIsLink, bool* aIsOnclick) } return walkUpAcc; } + + if (nsCoreUtils::IsLabelWithControl(walkUpAcc->GetContent())) { + if (aIsLabelWithControl) { + *aIsLabelWithControl = true; + } + return walkUpAcc; + } } return nullptr; } @@ -166,11 +177,11 @@ LinkableAccessible::ActionNameAt(uint8_t aIndex, nsAString& aName) // Action 0 (default action): Jump to link if (aIndex == eAction_Jump) { - bool isOnclick, isLink; - ActionWalk(&isLink, &isOnclick); + bool isOnclick, isLink, isLabelWithControl; + ActionWalk(&isLink, &isOnclick, &isLabelWithControl); if (isLink) { aName.AssignLiteral("jump"); - } else if (isOnclick) { + } else if (isOnclick || isLabelWithControl) { aName.AssignLiteral("click"); } } diff --git a/accessible/generic/BaseAccessibles.h b/accessible/generic/BaseAccessibles.h index 5e3c20e346..71e932949b 100644 --- a/accessible/generic/BaseAccessibles.h +++ b/accessible/generic/BaseAccessibles.h @@ -76,7 +76,8 @@ public: // ActionAccessible helpers Accessible* ActionWalk(bool* aIsLink = nullptr, - bool* aIsOnclick = nullptr); + bool* aIsOnclick = nullptr, + bool* aIsLabelWithControl = nullptr); // HyperLinkAccessible virtual already_AddRefed AnchorURIAt(uint32_t aAnchorIndex) override; diff --git a/accessible/html/HTMLElementAccessibles.cpp b/accessible/html/HTMLElementAccessibles.cpp index 02f31017d1..fa2cc4d350 100644 --- a/accessible/html/HTMLElementAccessibles.cpp +++ b/accessible/html/HTMLElementAccessibles.cpp @@ -75,6 +75,32 @@ HTMLLabelAccessible::RelationByType(RelationType aType) return rel; } +uint8_t +HTMLLabelAccessible::ActionCount() +{ + return nsCoreUtils::IsLabelWithControl(mContent) ? 1 : 0; +} + +void +HTMLLabelAccessible::ActionNameAt(uint8_t aIndex, nsAString& aName) +{ + if (aIndex == 0) { + if (nsCoreUtils::IsLabelWithControl(mContent)) + aName.AssignLiteral("click"); + } +} + +bool +HTMLLabelAccessible::DoAction(uint8_t aIndex) +{ + if (aIndex != 0) + return false; + + DoCommand(); + return true; +} + + //////////////////////////////////////////////////////////////////////////////// // nsHTMLOuputAccessible //////////////////////////////////////////////////////////////////////////////// diff --git a/accessible/html/HTMLElementAccessibles.h b/accessible/html/HTMLElementAccessibles.h index 8dfffd6f72..a1a4a8a9c4 100644 --- a/accessible/html/HTMLElementAccessibles.h +++ b/accessible/html/HTMLElementAccessibles.h @@ -62,6 +62,11 @@ public: // Accessible virtual Relation RelationByType(RelationType aType) override; + // ActionAccessible + virtual uint8_t ActionCount() override; + virtual void ActionNameAt(uint8_t aIndex, nsAString& aName) override; + virtual bool DoAction(uint8_t aIndex) override; + protected: virtual ~HTMLLabelAccessible() {} virtual ENameValueFlag NativeName(nsString& aName) override; diff --git a/accessible/tests/mochitest/actions/test_general.html b/accessible/tests/mochitest/actions/test_general.html index 1d312df302..5b9a18dab5 100644 --- a/accessible/tests/mochitest/actions/test_general.html +++ b/accessible/tests/mochitest/actions/test_general.html @@ -39,11 +39,19 @@ ID: "onclick_img", actionName: "click", events: CLICK_EVENTS + }, + { + ID: "label1", + actionName: "click", + events: CLICK_EVENTS } + ]; testActions(actionsArray); + is(getAccessible("label1").firstChild.actionCount, 1, "label text should have 1 action"); + getAccessible("onclick_img").takeFocus(); is(getAccessible("link1").actionCount, 1, "links should have one action"); is(getAccessible("link2").actionCount, 1, "link with onclick handler should have 1 action"); @@ -87,5 +95,13 @@ linkable textleaf accessible
linkable textleaf accessible
+ +
+ + +
+ diff --git a/accessible/tests/mochitest/actions/test_general.xul b/accessible/tests/mochitest/actions/test_general.xul index fac49f1bbb..bb27767a38 100644 --- a/accessible/tests/mochitest/actions/test_general.xul +++ b/accessible/tests/mochitest/actions/test_general.xul @@ -60,6 +60,11 @@ actionName: "press", events: CLICK_EVENTS }, + { + ID: "name_entry_label", + actionName: "click", + events: CLICK_EVENTS + }, { ID: "labelWithPopup", actionName: "click", @@ -72,6 +77,8 @@ }*/ ]; + is(getAccessible("name_entry_label").firstChild.actionCount, 1, "label text should have 1 action"); + testActions(actionsArray); } @@ -125,6 +132,10 @@