From e3e172b23fe72eb91abe26218826fe746771d49f Mon Sep 17 00:00:00 2001 From: Reef Turner Date: Tue, 9 Aug 2016 17:29:40 +0800 Subject: [PATCH 1/3] Dont use name as content when image has children Consider elements with `role == ROLE_SYSTEM_GRAPHIC` as image maps if they have children, and then render the children. Fix for #6051. --- .../vbufBackends/gecko_ia2/gecko_ia2.cpp | 57 ++++++++++++------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp b/nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp index 718ae759b2c..fb3491ae969 100755 --- a/nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp +++ b/nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp @@ -281,6 +281,22 @@ bool isLabelVisible(IAccessible2* acc) { return true; } +long getChildCount(const bool isAriaHidden, IAccessible2 * const pacc){ + long rawChildCount = 0; + if(!isAriaHidden){ + auto res = pacc->get_accChildCount(&rawChildCount); + if(res != S_OK){ + rawChildCount = 0; + } + } + return rawChildCount; +} + +bool hasAriaHiddenAttribute(map& IA2AttribsMap){ + auto IA2AttribsMapIt = IA2AttribsMap.find(L"hidden"); + return (IA2AttribsMapIt != IA2AttribsMap.end() && IA2AttribsMapIt->second == L"true"); +} + const vectorATTRLIST_ROLES(1, L"IAccessible2::attribute_xml-roles"); const wregex REGEX_PRESENTATION_ROLE(L"IAccessible2\\\\:\\\\:attribute_xml-roles:.*\\bpresentation\\b.*;"); @@ -487,11 +503,19 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(IAccessible2* pacc, // Certain objects are never interactive, even if other checks are true. bool isNeverInteractive = parentNode->isHidden||(!isEditable && (isRoot || role == ROLE_SYSTEM_DOCUMENT || role == IA2_ROLE_INTERNAL_FRAME)); bool isInteractive = !isNeverInteractive && (isEditable || inLink || states & STATE_SYSTEM_FOCUSABLE || states & STATE_SYSTEM_UNAVAILABLE || isEmbeddedApp || role == ROLE_SYSTEM_EQUATION); - // We aren't finished calculating isInteractive yet; actions are handled below. - // Whether the name is the content of this node. - bool nameIsContent = isEmbeddedApp - || role == ROLE_SYSTEM_LINK || role == ROLE_SYSTEM_PUSHBUTTON || role == IA2_ROLE_TOGGLE_BUTTON || role == ROLE_SYSTEM_MENUITEM || role == ROLE_SYSTEM_GRAPHIC || (role == ROLE_SYSTEM_TEXT && !isEditable) || role == IA2_ROLE_HEADING || role == ROLE_SYSTEM_PAGETAB || role == ROLE_SYSTEM_BUTTONMENU - || ((role == ROLE_SYSTEM_CHECKBUTTON || role == ROLE_SYSTEM_RADIOBUTTON) && !isLabelVisible(pacc)); + + const bool isAriaHidden = hasAriaHiddenAttribute(IA2AttribsMap); + const long childCount = getChildCount(isAriaHidden, pacc); + + bool nameIsContent = false; + { + const bool isImgMap = role == ROLE_SYSTEM_GRAPHIC && childCount > 0; + // We aren't finished calculating isInteractive yet; actions are handled below. + // Whether the name is the content of this node. + nameIsContent = isEmbeddedApp + || role == ROLE_SYSTEM_LINK || role == ROLE_SYSTEM_PUSHBUTTON || role == IA2_ROLE_TOGGLE_BUTTON || role == ROLE_SYSTEM_MENUITEM || (role == ROLE_SYSTEM_GRAPHIC && !isImgMap) || (role == ROLE_SYSTEM_TEXT && !isEditable) || role == IA2_ROLE_HEADING || role == ROLE_SYSTEM_PAGETAB || role == ROLE_SYSTEM_BUTTONMENU + || ((role == ROLE_SYSTEM_CHECKBUTTON || role == ROLE_SYSTEM_RADIOBUTTON) && !isLabelVisible(pacc)); + } IAccessibleText* paccText=NULL; IAccessibleHypertext* paccHypertext=NULL; @@ -523,12 +547,12 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(IAccessible2* pacc, // Whether to render children, including text content. // Note that we may still render the name, value, etc. even if we don't render children. bool renderChildren = true; - long childCount=0; - if ((IA2AttribsMapIt = IA2AttribsMap.find(L"hidden")) != IA2AttribsMap.end() && IA2AttribsMapIt->second == L"true") { + if (isAriaHidden) { // aria-hidden isVisible = false; } else { - isVisible = width > 0 && height > 0; + // If a node has children, it's visible. + isVisible = width > 0 && height > 0 || childCount > 0; if (IA2TextIsUnneededSpace || role == ROLE_SYSTEM_COMBOBOX || (role == ROLE_SYSTEM_LIST && !(states & STATE_SYSTEM_READONLY)) @@ -536,15 +560,9 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(IAccessible2* pacc, || role == ROLE_SYSTEM_OUTLINE || role == ROLE_SYSTEM_EQUATION || (nameIsContent && (IA2AttribsMapIt = IA2AttribsMap.find(L"explicit-name")) != IA2AttribsMap.end() && IA2AttribsMapIt->second == L"true") - ) + ) { renderChildren = false; - if(pacc->get_accChildCount(&childCount)==S_OK) { - if (childCount > 0) { - // If a node has children, it's visible. - isVisible = true; - } - } else - childCount=0; + } } //Expose all available actions @@ -733,11 +751,12 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(IAccessible2* pacc, LOG_DEBUG(L"Error allocating varChildren memory"); return NULL; } - if(AccessibleChildren(pacc,0,childCount,varChildren,&childCount)!=S_OK) { + long accessibleChildrenCount = 0; + if(AccessibleChildren(pacc,0,childCount,varChildren,&accessibleChildrenCount)!=S_OK) { LOG_DEBUG(L"AccessibleChildren failed"); - childCount=0; + accessibleChildrenCount=0; } - for(long i=0;i Date: Mon, 15 Aug 2016 11:47:58 +0800 Subject: [PATCH 2/3] Review actions for #6252 --- .../vbufBackends/gecko_ia2/gecko_ia2.cpp | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp b/nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp index fb3491ae969..40c2b4b8e5e 100755 --- a/nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp +++ b/nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp @@ -292,8 +292,8 @@ long getChildCount(const bool isAriaHidden, IAccessible2 * const pacc){ return rawChildCount; } -bool hasAriaHiddenAttribute(map& IA2AttribsMap){ - auto IA2AttribsMapIt = IA2AttribsMap.find(L"hidden"); +bool hasAriaHiddenAttribute(const map& IA2AttribsMap){ + const auto IA2AttribsMapIt = IA2AttribsMap.find(L"hidden"); return (IA2AttribsMapIt != IA2AttribsMap.end() && IA2AttribsMapIt->second == L"true"); } @@ -503,19 +503,19 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(IAccessible2* pacc, // Certain objects are never interactive, even if other checks are true. bool isNeverInteractive = parentNode->isHidden||(!isEditable && (isRoot || role == ROLE_SYSTEM_DOCUMENT || role == IA2_ROLE_INTERNAL_FRAME)); bool isInteractive = !isNeverInteractive && (isEditable || inLink || states & STATE_SYSTEM_FOCUSABLE || states & STATE_SYSTEM_UNAVAILABLE || isEmbeddedApp || role == ROLE_SYSTEM_EQUATION); + // We aren't finished calculating isInteractive yet; actions are handled below. const bool isAriaHidden = hasAriaHiddenAttribute(IA2AttribsMap); const long childCount = getChildCount(isAriaHidden, pacc); bool nameIsContent = false; - { - const bool isImgMap = role == ROLE_SYSTEM_GRAPHIC && childCount > 0; - // We aren't finished calculating isInteractive yet; actions are handled below. - // Whether the name is the content of this node. - nameIsContent = isEmbeddedApp - || role == ROLE_SYSTEM_LINK || role == ROLE_SYSTEM_PUSHBUTTON || role == IA2_ROLE_TOGGLE_BUTTON || role == ROLE_SYSTEM_MENUITEM || (role == ROLE_SYSTEM_GRAPHIC && !isImgMap) || (role == ROLE_SYSTEM_TEXT && !isEditable) || role == IA2_ROLE_HEADING || role == ROLE_SYSTEM_PAGETAB || role == ROLE_SYSTEM_BUTTONMENU - || ((role == ROLE_SYSTEM_CHECKBUTTON || role == ROLE_SYSTEM_RADIOBUTTON) && !isLabelVisible(pacc)); - } + + const bool isImgMap = role == ROLE_SYSTEM_GRAPHIC && childCount > 0; + // Whether the name is the content of this node. + nameIsContent = isEmbeddedApp + || role == ROLE_SYSTEM_LINK || role == ROLE_SYSTEM_PUSHBUTTON || role == IA2_ROLE_TOGGLE_BUTTON || role == ROLE_SYSTEM_MENUITEM || (role == ROLE_SYSTEM_GRAPHIC && !isImgMap) || (role == ROLE_SYSTEM_TEXT && !isEditable) || role == IA2_ROLE_HEADING || role == ROLE_SYSTEM_PAGETAB || role == ROLE_SYSTEM_BUTTONMENU + || ((role == ROLE_SYSTEM_CHECKBUTTON || role == ROLE_SYSTEM_RADIOBUTTON) && !isLabelVisible(pacc)); + IAccessibleText* paccText=NULL; IAccessibleHypertext* paccHypertext=NULL; @@ -667,7 +667,7 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(IAccessible2* pacc, parentNode->addAttribute(L"name", name); if (isVisible) { - if (role==ROLE_SYSTEM_GRAPHIC&&childCount>0&&name) { + if ( isImgMap && name ) { // This is an image map with a name. Render the name first. previousNode=buffer->addTextFieldNode(parentNode,previousNode,name); if(previousNode&&!locale.empty()) previousNode->addAttribute(L"language",locale); From f5794375fa764815b7e3febed0fdc107e1069755 Mon Sep 17 00:00:00 2001 From: Reef Turner Date: Tue, 16 Aug 2016 13:56:40 +0800 Subject: [PATCH 3/3] Further review actions for #6252 --- nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp b/nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp index 40c2b4b8e5e..f3caf57e3a5 100755 --- a/nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp +++ b/nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp @@ -508,11 +508,9 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(IAccessible2* pacc, const bool isAriaHidden = hasAriaHiddenAttribute(IA2AttribsMap); const long childCount = getChildCount(isAriaHidden, pacc); - bool nameIsContent = false; - const bool isImgMap = role == ROLE_SYSTEM_GRAPHIC && childCount > 0; // Whether the name is the content of this node. - nameIsContent = isEmbeddedApp + const bool nameIsContent = isEmbeddedApp || role == ROLE_SYSTEM_LINK || role == ROLE_SYSTEM_PUSHBUTTON || role == IA2_ROLE_TOGGLE_BUTTON || role == ROLE_SYSTEM_MENUITEM || (role == ROLE_SYSTEM_GRAPHIC && !isImgMap) || (role == ROLE_SYSTEM_TEXT && !isEditable) || role == IA2_ROLE_HEADING || role == ROLE_SYSTEM_PAGETAB || role == ROLE_SYSTEM_BUTTONMENU || ((role == ROLE_SYSTEM_CHECKBUTTON || role == ROLE_SYSTEM_RADIOBUTTON) && !isLabelVisible(pacc));