From ddc5be8ef1a3d8d51a09b8b50abbc84acf267410 Mon Sep 17 00:00:00 2001 From: Alexander Surkov Date: Sat, 28 Feb 2015 17:25:06 -0500 Subject: [PATCH] Bug 1137714 - Make roleDescription nicer/correct/faster, r=marcoz --- accessible/base/ARIAMap.cpp | 68 ++++++++++++-- accessible/base/AccTypes.h | 15 ++-- accessible/base/nsAccUtils.cpp | 11 ++- accessible/base/nsAccUtils.h | 4 + accessible/generic/Accessible-inl.h | 9 ++ accessible/generic/Accessible.cpp | 23 +++-- accessible/generic/Accessible.h | 9 +- accessible/generic/HyperTextAccessible.cpp | 75 +++++++++------- accessible/generic/HyperTextAccessible.h | 1 + accessible/mac/mozAccessible.mm | 100 ++++++++++----------- dom/base/nsGkAtomList.h | 5 ++ 11 files changed, 216 insertions(+), 104 deletions(-) diff --git a/accessible/base/ARIAMap.cpp b/accessible/base/ARIAMap.cpp index 76d0972e015d..cb285c4d3ace 100644 --- a/accessible/base/ARIAMap.cpp +++ b/accessible/base/ARIAMap.cpp @@ -32,10 +32,6 @@ static const uint32_t kGenericAccType = 0; * * When no Role enum mapping exists for an ARIA role, the role will be exposed * via the object attribute "xml-roles". - * - * There are no Role enums for the following landmark roles: - * banner, contentinfo, main, navigation, note, search, secondary, - * seealso, breadcrumbs. */ static nsRoleMapEntry sWAIRoleMaps[] = @@ -67,7 +63,7 @@ static nsRoleMapEntry sWAIRoleMaps[] = eNoValue, eNoAction, eNoLiveAttr, - kGenericAccType, + eLandmark, kNoReqStates }, { // article @@ -81,6 +77,16 @@ static nsRoleMapEntry sWAIRoleMaps[] = kNoReqStates, eReadonlyUntilEditable }, + { // banner + &nsGkAtoms::banner, + roles::NOTHING, + kUseNativeRole, + eNoValue, + eNoAction, + eNoLiveAttr, + eLandmark, + kNoReqStates + }, { // button &nsGkAtoms::button, roles::PUSHBUTTON, @@ -129,6 +135,26 @@ static nsRoleMapEntry sWAIRoleMaps[] = eARIAReadonly, eARIAOrientation }, + { // complementary + &nsGkAtoms::complementary, + roles::NOTHING, + kUseNativeRole, + eNoValue, + eNoAction, + eNoLiveAttr, + eLandmark, + kNoReqStates + }, + { // contentinfo + &nsGkAtoms::contentinfo, + roles::NOTHING, + kUseNativeRole, + eNoValue, + eNoAction, + eNoLiveAttr, + eLandmark, + kNoReqStates + }, { // dialog &nsGkAtoms::dialog, roles::DIALOG, @@ -167,7 +193,7 @@ static nsRoleMapEntry sWAIRoleMaps[] = eNoValue, eNoAction, eNoLiveAttr, - kGenericAccType, + eLandmark, kNoReqStates }, { // grid @@ -290,6 +316,16 @@ static nsRoleMapEntry sWAIRoleMaps[] = kGenericAccType, kNoReqStates }, + { // main + &nsGkAtoms::main, + roles::NOTHING, + kUseNativeRole, + eNoValue, + eNoAction, + eNoLiveAttr, + eLandmark, + kNoReqStates + }, { // marquee &nsGkAtoms::marquee, roles::ANIMATION, @@ -366,6 +402,16 @@ static nsRoleMapEntry sWAIRoleMaps[] = kNoReqStates, eARIACheckableBool }, + { // navigation + &nsGkAtoms::navigation, + roles::NOTHING, + kUseNativeRole, + eNoValue, + eNoAction, + eNoLiveAttr, + eLandmark, + kNoReqStates + }, { // none &nsGkAtoms::none, roles::NOTHING, @@ -496,6 +542,16 @@ static nsRoleMapEntry sWAIRoleMaps[] = eARIAOrientation, eARIAReadonly }, + { // search + &nsGkAtoms::search, + roles::NOTHING, + kUseNativeRole, + eNoValue, + eNoAction, + eNoLiveAttr, + eLandmark, + kNoReqStates + }, { // searchbox &nsGkAtoms::searchbox, roles::ENTRY, diff --git a/accessible/base/AccTypes.h b/accessible/base/AccTypes.h index b1857173745b..aa0582adb5c6 100644 --- a/accessible/base/AccTypes.h +++ b/accessible/base/AccTypes.h @@ -74,13 +74,14 @@ enum AccGenericType { eCombobox = 1 << 3, eDocument = 1 << 4, eHyperText = 1 << 5, - eList = 1 << 6, - eListControl = 1 << 7, - eMenuButton = 1 << 8, - eSelect = 1 << 9, - eTable = 1 << 10, - eTableCell = 1 << 11, - eTableRow = 1 << 12, + eLandmark = 1 << 6, + eList = 1 << 7, + eListControl = 1 << 8, + eMenuButton = 1 << 9, + eSelect = 1 << 10, + eTable = 1 << 11, + eTableCell = 1 << 12, + eTableRow = 1 << 13, eLastAccGenericType = eTableRow }; diff --git a/accessible/base/nsAccUtils.cpp b/accessible/base/nsAccUtils.cpp index d709388a6f26..c4b4aa26fb72 100644 --- a/accessible/base/nsAccUtils.cpp +++ b/accessible/base/nsAccUtils.cpp @@ -37,11 +37,18 @@ nsAccUtils::SetAccAttr(nsIPersistentProperties *aAttributes, nsIAtom *aAttrName, const nsAString& aAttrValue) { nsAutoString oldValue; - nsAutoCString attrName; - aAttributes->SetStringProperty(nsAtomCString(aAttrName), aAttrValue, oldValue); } +void +nsAccUtils::SetAccAttr(nsIPersistentProperties *aAttributes, + nsIAtom* aAttrName, nsIAtom* aAttrValue) +{ + nsAutoString oldValue; + aAttributes->SetStringProperty(nsAtomCString(aAttrName), + nsAtomString(aAttrValue), oldValue); +} + void nsAccUtils::SetAccGroupAttrs(nsIPersistentProperties *aAttributes, int32_t aLevel, int32_t aSetSize, diff --git a/accessible/base/nsAccUtils.h b/accessible/base/nsAccUtils.h index 6b939d2d9ec5..9cd57c140635 100644 --- a/accessible/base/nsAccUtils.h +++ b/accessible/base/nsAccUtils.h @@ -50,6 +50,10 @@ public: nsIAtom *aAttrName, const nsAString& aAttrValue); + static void SetAccAttr(nsIPersistentProperties *aAttributes, + nsIAtom* aAttrName, + nsIAtom* aAttrValue); + /** * Set group attributes ('level', 'setsize', 'posinset'). */ diff --git a/accessible/generic/Accessible-inl.h b/accessible/generic/Accessible-inl.h index cfe4eece79f3..460f60c34c22 100644 --- a/accessible/generic/Accessible-inl.h +++ b/accessible/generic/Accessible-inl.h @@ -44,6 +44,15 @@ Accessible::ARIARole() return ARIATransformRole(mRoleMapEntry->role); } +inline bool +Accessible::IsSearchbox() const +{ + return (mRoleMapEntry && mRoleMapEntry->Is(nsGkAtoms::searchbox)) || + (mContent->IsHTML(nsGkAtoms::input) && + mContent->AttrValueIs(kNameSpaceID_None, nsGkAtoms::type, + nsGkAtoms::textInputType, eCaseMatters)); +} + inline bool Accessible::HasGenericType(AccGenericType aType) const { diff --git a/accessible/generic/Accessible.cpp b/accessible/generic/Accessible.cpp index 49e6759a848f..045f801e1f75 100644 --- a/accessible/generic/Accessible.cpp +++ b/accessible/generic/Accessible.cpp @@ -860,14 +860,20 @@ Accessible::Attributes() if (!HasOwnContent() || !mContent->IsElement()) return attributes.forget(); - // 'xml-roles' attribute coming from ARIA. - nsAutoString xmlRoles, unused; - if (mContent->GetAttr(kNameSpaceID_None, nsGkAtoms::role, xmlRoles)) { - attributes->SetStringProperty(NS_LITERAL_CSTRING("xml-roles"), - xmlRoles, unused); + // 'xml-roles' attribute for landmark. + nsIAtom* landmark = LandmarkRole(); + if (landmark) { + nsAccUtils::SetAccAttr(attributes, nsGkAtoms::xmlroles, landmark); + + } else { + // 'xml-roles' attribute coming from ARIA. + nsAutoString xmlRoles; + if (mContent->GetAttr(kNameSpaceID_None, nsGkAtoms::role, xmlRoles)) + nsAccUtils::SetAccAttr(attributes, nsGkAtoms::xmlroles, xmlRoles); } // Expose object attributes from ARIA attributes. + nsAutoString unused; aria::AttrIterator attribIter(mContent); nsAutoString name, value; while(attribIter.Next(name, value)) @@ -1390,6 +1396,13 @@ Accessible::ARIATransformRole(role aRole) return aRole; } +nsIAtom* +Accessible::LandmarkRole() const +{ + return mRoleMapEntry && mRoleMapEntry->IsOfType(eLandmark) ? + *(mRoleMapEntry->roleAtom) : nullptr; +} + role Accessible::NativeRole() { diff --git a/accessible/generic/Accessible.h b/accessible/generic/Accessible.h index db015daea7ea..d9d0dbab6d9d 100644 --- a/accessible/generic/Accessible.h +++ b/accessible/generic/Accessible.h @@ -230,6 +230,11 @@ public: */ mozilla::a11y::role ARIARole(); + /** + * Return a landmark role if applied. + */ + virtual nsIAtom* LandmarkRole() const; + /** * Returns enumerated accessible role from native markup (see constants in * Role.h). Doesn't take into account ARIA roles. @@ -621,6 +626,8 @@ public: bool IsRoot() const { return mType == eRootType; } a11y::RootAccessible* AsRoot(); + bool IsSearchbox() const; + bool IsSelect() const { return HasGenericType(eSelect); } bool IsTable() const { return HasGenericType(eTable); } @@ -1098,7 +1105,7 @@ protected: static const uint8_t kStateFlagsBits = 9; static const uint8_t kContextFlagsBits = 2; static const uint8_t kTypeBits = 6; - static const uint8_t kGenericTypesBits = 13; + static const uint8_t kGenericTypesBits = 14; /** * Keep in sync with ChildrenFlags, StateFlags, ContextFlags, and AccTypes. diff --git a/accessible/generic/HyperTextAccessible.cpp b/accessible/generic/HyperTextAccessible.cpp index 7337d261dc5f..29540b67bab8 100644 --- a/accessible/generic/HyperTextAccessible.cpp +++ b/accessible/generic/HyperTextAccessible.cpp @@ -944,45 +944,13 @@ HyperTextAccessible::NativeAttributes() if (!HasOwnContent()) return attributes.forget(); - // For the html landmark elements we expose them like we do aria landmarks to - // make AT navigation schemes "just work". nsIAtom* tag = mContent->Tag(); - if (tag == nsGkAtoms::nav) { - nsAccUtils::SetAccAttr(attributes, nsGkAtoms::xmlroles, - NS_LITERAL_STRING("navigation")); - } else if (tag == nsGkAtoms::section) { + if (tag == nsGkAtoms::section) { nsAccUtils::SetAccAttr(attributes, nsGkAtoms::xmlroles, NS_LITERAL_STRING("region")); - } else if (tag == nsGkAtoms::header || tag == nsGkAtoms::footer) { - // Only map header and footer if they are not descendants - // of an article or section tag. - nsIContent* parent = mContent->GetParent(); - while (parent) { - if (parent->Tag() == nsGkAtoms::article || - parent->Tag() == nsGkAtoms::section) - break; - parent = parent->GetParent(); - } - - // No article or section elements found. - if (!parent) { - if (tag == nsGkAtoms::header) { - nsAccUtils::SetAccAttr(attributes, nsGkAtoms::xmlroles, - NS_LITERAL_STRING("banner")); - } else if (tag == nsGkAtoms::footer) { - nsAccUtils::SetAccAttr(attributes, nsGkAtoms::xmlroles, - NS_LITERAL_STRING("contentinfo")); - } - } - } else if (tag == nsGkAtoms::aside) { - nsAccUtils::SetAccAttr(attributes, nsGkAtoms::xmlroles, - NS_LITERAL_STRING("complementary")); } else if (tag == nsGkAtoms::article) { nsAccUtils::SetAccAttr(attributes, nsGkAtoms::xmlroles, NS_LITERAL_STRING("article")); - } else if (tag == nsGkAtoms::main) { - nsAccUtils::SetAccAttr(attributes, nsGkAtoms::xmlroles, - NS_LITERAL_STRING("main")); } else if (tag == nsGkAtoms::time) { nsAccUtils::SetAccAttr(attributes, nsGkAtoms::xmlroles, NS_LITERAL_STRING("time")); @@ -997,6 +965,47 @@ HyperTextAccessible::NativeAttributes() return attributes.forget(); } +nsIAtom* +HyperTextAccessible::LandmarkRole() const +{ + // For the html landmark elements we expose them like we do ARIA landmarks to + // make AT navigation schemes "just work". + nsIAtom* tag = mContent->Tag(); + if (tag == nsGkAtoms::nav) + return nsGkAtoms::navigation; + + if (tag == nsGkAtoms::header || tag == nsGkAtoms::footer) { + // Only map header and footer if they are not descendants of an article + // or section tag. + nsIContent* parent = mContent->GetParent(); + while (parent) { + if (parent->Tag() == nsGkAtoms::article || + parent->Tag() == nsGkAtoms::section) + break; + parent = parent->GetParent(); + } + + // No article or section elements found. + if (!parent) { + if (tag == nsGkAtoms::header) + return nsGkAtoms::banner; + + if (tag == nsGkAtoms::footer) { + return nsGkAtoms::contentinfo; + } + } + return nullptr; + } + + if (tag == nsGkAtoms::aside) + return nsGkAtoms::complementary; + + if (tag == nsGkAtoms::main) + return nsGkAtoms::main; + + return nullptr; +} + int32_t HyperTextAccessible::OffsetAtPoint(int32_t aX, int32_t aY, uint32_t aCoordType) { diff --git a/accessible/generic/HyperTextAccessible.h b/accessible/generic/HyperTextAccessible.h index 0d8bec692d77..e4dd2aaf9f73 100644 --- a/accessible/generic/HyperTextAccessible.h +++ b/accessible/generic/HyperTextAccessible.h @@ -54,6 +54,7 @@ public: NS_DECL_ISUPPORTS_INHERITED // Accessible + virtual nsIAtom* LandmarkRole() const MOZ_OVERRIDE; virtual int32_t GetLevelInternal() MOZ_OVERRIDE; virtual already_AddRefed NativeAttributes() MOZ_OVERRIDE; virtual mozilla::a11y::role NativeRole() MOZ_OVERRIDE; diff --git a/accessible/mac/mozAccessible.mm b/accessible/mac/mozAccessible.mm index 1f078c9740f0..12e888577fd3 100644 --- a/accessible/mac/mozAccessible.mm +++ b/accessible/mac/mozAccessible.mm @@ -430,38 +430,25 @@ GetClosestInterestingAccessible(id anObject) if (!mGeckoAccessible) return nil; - // XXX maybe we should cache the subrole. - nsAutoString xmlRoles; - - // XXX we don't need all the attributes (see bug 771113) - nsCOMPtr attributes = mGeckoAccessible->Attributes(); - if (attributes) - nsAccUtils::GetAccAttr(attributes, nsGkAtoms::xmlroles, xmlRoles); - - nsWhitespaceTokenizer tokenizer(xmlRoles); - - while (tokenizer.hasMoreTokens()) { - const nsDependentSubstring token(tokenizer.nextToken()); - - if (token.EqualsLiteral("banner")) + nsIAtom* landmark = mGeckoAccessible->LandmarkRole(); + if (landmark) { + if (landmark == nsGkAtoms::application) + return @"AXLandmarkApplication"; + if (landmark == nsGkAtoms::banner) return @"AXLandmarkBanner"; - - if (token.EqualsLiteral("complementary")) + if (landmark == nsGkAtoms::complementary) return @"AXLandmarkComplementary"; - - if (token.EqualsLiteral("contentinfo")) + if (landmark == nsGkAtoms::contentinfo) return @"AXLandmarkContentInfo"; - - if (token.EqualsLiteral("main")) + if (landmark == nsGkAtoms::form) + return @"AXLandmarkForm"; + if (landmark == nsGkAtoms::main) return @"AXLandmarkMain"; - - if (token.EqualsLiteral("navigation")) + if (landmark == nsGkAtoms::navigation) return @"AXLandmarkNavigation"; - - if (token.EqualsLiteral("search")) + if (landmark == nsGkAtoms::search) return @"AXLandmarkSearch"; - - if (token.EqualsLiteral("searchbox")) + if (landmark == nsGkAtoms::searchbox) return @"AXSearchField"; } @@ -469,6 +456,11 @@ GetClosestInterestingAccessible(id anObject) case roles::LIST: return @"AXContentList"; // 10.6+ NSAccessibilityContentListSubrole; + case roles::ENTRY: + if (mGeckoAccessible->IsSearchbox()) + return @"AXSearchField"; + break; + case roles::DEFINITION_LIST: return @"AXDefinitionList"; // 10.6+ NSAccessibilityDefinitionListSubrole; @@ -488,6 +480,33 @@ GetClosestInterestingAccessible(id anObject) return nil; } +struct RoleDescrMap +{ + NSString* role; + const nsString& description; +}; + +static const RoleDescrMap sRoleDescrMap[] = { + { @"AXDefinition", NS_LITERAL_STRING("definition") }, + { @"AXLandmarkBanner", NS_LITERAL_STRING("banner") }, + { @"AXLandmarkComplementary", NS_LITERAL_STRING("complementary") }, + { @"AXLandmarkContentInfo", NS_LITERAL_STRING("content") }, + { @"AXLandmarkMain", NS_LITERAL_STRING("main") }, + { @"AXLandmarkNavigation", NS_LITERAL_STRING("navigation") }, + { @"AXLandmarkSearch", NS_LITERAL_STRING("search") }, + { @"AXSearchField", NS_LITERAL_STRING("searchTextField") }, + { @"AXTerm", NS_LITERAL_STRING("term") } +}; + +struct RoleDescrComparator +{ + const NSString* mRole; + explicit RoleDescrComparator(const NSString* aRole) : mRole(aRole) {} + int operator()(const RoleDescrMap& aEntry) const { + return [mRole compare:aEntry.role]; + } +}; + - (NSString*)roleDescription { if (mRole == roles::DOCUMENT) @@ -495,32 +514,13 @@ GetClosestInterestingAccessible(id anObject) NSString* subrole = [self subrole]; - if ((mRole == roles::LISTITEM) && [subrole isEqualToString:@"AXTerm"]) - return utils::LocalizedString(NS_LITERAL_STRING("term")); - if ((mRole == roles::PARAGRAPH) && [subrole isEqualToString:@"AXDefinition"]) - return utils::LocalizedString(NS_LITERAL_STRING("definition")); - if ((mRole == roles::ENTRY) && [subrole isEqualToString:@"AXSearchField"]) - return utils::LocalizedString(NS_LITERAL_STRING("searchTextField")); - - NSString* role = [self role]; - - // the WAI-ARIA Landmarks - if ([role isEqualToString:NSAccessibilityGroupRole]) { - if ([subrole isEqualToString:@"AXLandmarkBanner"]) - return utils::LocalizedString(NS_LITERAL_STRING("banner")); - if ([subrole isEqualToString:@"AXLandmarkComplementary"]) - return utils::LocalizedString(NS_LITERAL_STRING("complementary")); - if ([subrole isEqualToString:@"AXLandmarkContentInfo"]) - return utils::LocalizedString(NS_LITERAL_STRING("content")); - if ([subrole isEqualToString:@"AXLandmarkMain"]) - return utils::LocalizedString(NS_LITERAL_STRING("main")); - if ([subrole isEqualToString:@"AXLandmarkNavigation"]) - return utils::LocalizedString(NS_LITERAL_STRING("navigation")); - if ([subrole isEqualToString:@"AXLandmarkSearch"]) - return utils::LocalizedString(NS_LITERAL_STRING("search")); + size_t idx = 0; + if (BinarySearchIf(sRoleDescrMap, 0, ArrayLength(sRoleDescrMap), + RoleDescrComparator(subrole), &idx)) { + return utils::LocalizedString(sRoleDescrMap[idx].description); } - return NSAccessibilityRoleDescription(role, subrole); + return NSAccessibilityRoleDescription([self role], subrole); } - (NSString*)title diff --git a/dom/base/nsGkAtomList.h b/dom/base/nsGkAtomList.h index e7cb2d3c9a15..651eaca938a4 100644 --- a/dom/base/nsGkAtomList.h +++ b/dom/base/nsGkAtomList.h @@ -2256,14 +2256,17 @@ GK_ATOM(aria_valuemax, "aria-valuemax") GK_ATOM(aria_valuetext, "aria-valuetext") GK_ATOM(AreaFrame, "AreaFrame") GK_ATOM(auto_generated, "auto-generated") +GK_ATOM(banner, "banner") GK_ATOM(checkable, "checkable") GK_ATOM(choices, "choices") GK_ATOM(columnheader, "columnheader") +GK_ATOM(complementary, "complementary") GK_ATOM(containerAtomic, "container-atomic") GK_ATOM(containerBusy, "container-busy") GK_ATOM(containerLive, "container-live") GK_ATOM(containerLiveRole, "container-live-role") GK_ATOM(containerRelevant, "container-relevant") +GK_ATOM(contentinfo, "contentinfo") GK_ATOM(cycles, "cycles") GK_ATOM(datatable, "datatable") GK_ATOM(directory, "directory") @@ -2284,6 +2287,7 @@ GK_ATOM(menuitemcheckbox, "menuitemcheckbox") GK_ATOM(menuitemradio, "menuitemradio") GK_ATOM(mixed, "mixed") GK_ATOM(multiline, "multiline") +GK_ATOM(navigation, "navigation") GK_ATOM(password, "password") GK_ATOM(posinset, "posinset") GK_ATOM(presentation, "presentation") @@ -2291,6 +2295,7 @@ GK_ATOM(progressbar, "progressbar") GK_ATOM(region, "region") GK_ATOM(rowgroup, "rowgroup") GK_ATOM(rowheader, "rowheader") +GK_ATOM(search, "search") GK_ATOM(searchbox, "searchbox") GK_ATOM(select1, "select1") GK_ATOM(setsize, "setsize")