Skip to content

Commit 5f86055

Browse files
seanbuddseanbudd
authored
Merge af8f9a3 into b53692a 
2 parents b53692a + af8f9a3 commit 5f86055

15 files changed

Lines changed: 910 additions & 307 deletions

File tree

‎nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp‎

Lines changed: 139 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/*
22
This file is a part of the NVDA project.
33
URL: http://www.nvda-project.org/
4-
Copyright 2007-2022 NV Access Limited, Mozilla Corporation
4+
Copyright 2007-2023 NV Access Limited, Mozilla Corporation
55
This program is free software: you can redistribute it and/or modify
66
it under the terms of the GNU General Public License version 2.0, as published by
77
the Free Software Foundation.
@@ -13,6 +13,7 @@ This license can be found at:
1313
*/
1414

1515
#include <memory>
16+
#include <numeric>
1617
#include <functional>
1718
#include <vector>
1819
#include <map>
@@ -47,43 +48,73 @@ bool hasXmlRoleAttribContainingValue(const map<wstring,wstring>& attribsMap, con
4748
return attribsMapIt != attribsMap.end() && attribsMapIt->second.find(roleName) != wstring::npos;
4849
}
4950

50-
CComPtr<IAccessible2> GeckoVBufBackend_t::getRelationElement(
51+
std::vector<CComQIPtr<IAccessible2>> GeckoVBufBackend_t::getRelationElementsOfType(
5152
LPCOLESTR ia2TargetRelation,
52-
IAccessible2_2* element
53+
IAccessible2_2* element,
54+
const std::optional<std::size_t> numRelations
5355
) {
54-
IUnknown** ppUnk=nullptr;
55-
long nTargets=0;
56-
// We only need to request one relation target
57-
int numRelations=1;
58-
// However, a bug in Chrome causes a buffer overrun if numRelations is less than the total number of targets the node has.
59-
// Therefore, If this is Chrome, request all targets (by setting numRelations to 0) as this works around the bug.
60-
// There is no major performance hit to fetch all targets in Chrome as Chrome is already fetching all targets either way.
61-
// In Firefox there would be extra cross-proc calls.
62-
if(this->toolkitName.compare(L"Chrome")==0) {
63-
numRelations=0;
56+
constexpr long FETCH_ALL = 0l; // See docs of relationTargetsOfType.
57+
long nTargets = static_cast<long>(
58+
min<size_t>(
59+
numRelations.value_or(FETCH_ALL), // Default is to fetch all relations.
60+
static_cast<size_t>((numeric_limits<long>::max)())
61+
)
62+
);
63+
64+
const bool isChrome = this->toolkitName.compare(L"Chrome") == 0;
65+
if (isChrome) {
66+
// A bug in Chrome causes a buffer overrun if numRelations is less than the total number of targets the node has.
67+
// TODO: has this been reported?
68+
// numRelations cannot be correctly determined due to another bug: https://crbug.com/1399184
69+
// As a work around, request all targets (by setting numRelations to 0).
70+
// There is no major performance hit to fetch all targets in Chrome as Chrome is already fetching all targets either way.
71+
// In Firefox there would be extra cross-process calls.
72+
nTargets = FETCH_ALL;
6473
}
6574
// the relation type string *must* be passed correctly as a BSTR otherwise we can see crashes in 32 bit Firefox.
6675
CComBSTR relationAsBSTR(ia2TargetRelation);
76+
77+
IUnknown** targets = nullptr;
78+
/*
79+
relationTargetsOfType(
80+
// type: use IA2_RELATION_* constants
81+
// from 'include/ia2/api/AccessibleRelation.idl' becomes 'build/<arch>/ia2.h'
82+
[in] BSTR type,
83+
[in] long maxTargets, // 0 means all.
84+
[out, size_is(,*nTargets)] IUnknown ***targets, // targets data.
85+
[out, retval] long *nTargets // number targets fetched
86+
)*/
6787
HRESULT res = element->get_relationTargetsOfType(
6888
relationAsBSTR,
69-
numRelations,
70-
&ppUnk,
89+
nTargets,
90+
&targets,
7191
&nTargets
7292
);
73-
if(res!=S_OK) return nullptr;
74-
// Grab all the returned IUnknowns and store them as smart pointers within a smart pointer array
75-
// so that any further returns will correctly release all the objects.
76-
auto ppUnk_smart=make_unique<CComPtr<IUnknown>[]>(nTargets);
77-
for(int i=0;i<nTargets;++i) {
78-
ppUnk_smart[i].Attach(ppUnk[i]);
93+
if (res != S_OK) {
94+
// Return early on failure. Due to failure, there is no need to CoTaskMemFree
95+
// the targets parameter can be considered invalid.
96+
return {};
97+
}
98+
// Grab all the returned IUnknowns and store them as a collection of smart pointers
99+
// so that any further returns will correctly release all the objects.
100+
const std::size_t numFetched = static_cast<std::size_t>(nTargets >= 0 ? nTargets : 0); // handle negative numbers for signed conversion.
101+
const std::size_t numToReturn = std::min<std::size_t>( // Limit to numRelations, the amount expected by the caller.
102+
numRelations.value_or(numFetched),
103+
numFetched
104+
);
105+
std::vector< CComQIPtr<IAccessible2>> smartTargets;
106+
for(std::size_t i = 0; i < numToReturn; ++i) {
107+
CComPtr<IUnknown> punk;
108+
punk.Attach(targets[i]);
109+
smartTargets.emplace_back(punk);
79110
}
80111
// we can now free the memory that Gecko allocated to give us the IUnknowns
81-
CoTaskMemFree(ppUnk);
112+
CoTaskMemFree(targets);
82113
if(nTargets==0) {
83114
LOG_DEBUG(L"relationTargetsOfType for " << relationAsBSTR.m_str << L" found no targets");
84-
return nullptr;
115+
return {};
85116
}
86-
return CComQIPtr<IAccessible2>(ppUnk_smart[0]);
117+
return smartTargets;
87118
}
88119

89120
const wchar_t EMBEDDED_OBJ_CHAR = 0xFFFC;
@@ -220,11 +251,16 @@ void GeckoVBufBackend_t::versionSpecificInit(IAccessible2* pacc) {
220251

221252
optional<int>
222253
getIAccessible2UniqueID(IAccessible2* targetAcc) {
223-
int ID = 0;
254+
if (targetAcc == nullptr) {
255+
LOG_ERROR(L"targetAcc is null, can't getIAccessible2UniqueID");
256+
return {};
257+
}
258+
224259
//Get ID -- IAccessible2 uniqueID
225-
if (targetAcc->get_uniqueID((long*)&ID) != S_OK) {
226-
LOG_DEBUG(L"pacc->get_uniqueID failed");
227-
return optional<int>();
260+
long ID = 0l;
261+
if (targetAcc->get_uniqueID(&ID) != S_OK) {
262+
LOG_DEBUG(L"targetAcc->get_uniqueID failed");
263+
return {};
228264
}
229265
return ID;
230266
}
@@ -239,25 +275,38 @@ using OptionalLabelInfo = optional< LabelInfo >;
239275
OptionalLabelInfo GeckoVBufBackend_t::getLabelInfo(IAccessible2* pacc2) {
240276
CComQIPtr<IAccessible2_2> pacc2_2=pacc2;
241277
if (!pacc2_2) return OptionalLabelInfo();
242-
auto targetAcc = getRelationElement(IA2_RELATION_LABELLED_BY, pacc2_2);
243-
if(!targetAcc) return OptionalLabelInfo();
278+
constexpr std::size_t maxRelationsToFetch = 1;
279+
auto accTargets = getRelationElementsOfType(IA2_RELATION_LABELLED_BY, pacc2_2, maxRelationsToFetch);
280+
281+
if (1 > accTargets.size()) {
282+
return OptionalLabelInfo();
283+
}
284+
auto firstAccTarget = accTargets[0];
244285
CComVariant child;
245286
child.vt = VT_I4;
246287
child.lVal = 0;
247288
CComVariant state;
248-
HRESULT res = targetAcc->get_accState(child, &state);
289+
HRESULT res = firstAccTarget->get_accState(child, &state);
249290
bool isVisible = res == S_OK && !(state.lVal & STATE_SYSTEM_INVISIBLE);
250-
auto ID = getIAccessible2UniqueID(targetAcc);
291+
auto ID = getIAccessible2UniqueID(firstAccTarget);
251292
return LabelInfo { isVisible, ID } ;
252293
}
253294

254-
std::optional<int> GeckoVBufBackend_t::getRelationId(LPCOLESTR ia2TargetRelation, IAccessible2* pacc2) {
295+
std::vector<int> GeckoVBufBackend_t::getAllRelationIdsForRelationType(LPCOLESTR ia2TargetRelation, IAccessible2* pacc2) {
255296
CComQIPtr<IAccessible2_2> pacc2_2 = pacc2;
256-
if (pacc2_2 == nullptr) return std::optional<int>();
257-
auto targetAcc = getRelationElement(ia2TargetRelation, pacc2_2);
258-
if (targetAcc == nullptr) return std::optional<int>();
259-
auto ID = getIAccessible2UniqueID(targetAcc);
260-
return ID;
297+
if (pacc2_2 == nullptr) {
298+
return {};
299+
}
300+
auto accTargets = getRelationElementsOfType(ia2TargetRelation, pacc2_2);
301+
std::vector<int> ids;
302+
for (auto& accTarget : accTargets) {
303+
auto ID = getIAccessible2UniqueID(accTarget);
304+
if (ID.has_value()) {
305+
ids.push_back(ID.value());
306+
}
307+
}
308+
309+
return ids;
261310
}
262311

263312
long getChildCount(const bool isAriaHidden, IAccessible2 * const pacc){
@@ -388,33 +437,66 @@ const vector<wstring>ATTRLIST_ROLES(1, L"IAccessible2::attribute_xml-roles");
388437
const wregex REGEX_PRESENTATION_ROLE(L"IAccessible2\\\\:\\\\:attribute_xml-roles:.*\\bpresentation\\b.*;");
389438

390439

440+
void _extendDetailsRolesAttribute(VBufStorage_controlFieldNode_t& node, const std::wstring& detailsRole)
441+
{
442+
std::wstringstream ss;
443+
auto roles = node.getAttribute(L"detailsRoles");
444+
if (roles) {
445+
ss << *roles << ',' << detailsRole;
446+
}
447+
else {
448+
ss << detailsRole;
449+
}
450+
node.addAttribute(L"detailsRoles", ss.str()); // addAttribute will replace an attribute that already exists
451+
}
452+
391453
void GeckoVBufBackend_t::fillVBufAriaDetails(
392454
int docHandle,
393455
CComPtr<IAccessible2> pacc,
394456
VBufStorage_buffer_t& buffer,
395-
VBufStorage_controlFieldNode_t& parentNode,
396-
const std::wstring& roleAttr
457+
VBufStorage_controlFieldNode_t& nodeBeingFilled,
458+
const std::wstring& nodeBeingFilledRole
397459
){
398460
/* Set the details role by checking for both IA2_RELATION_DETAILS and IA2_RELATION_DETAILS_FOR as one
399-
of the nodes in the relationship will not be in the buffer yet */
400-
std::optional<int> detailsId = getRelationId(IA2_RELATION_DETAILS, pacc);
401-
std::optional<int> detailsForId = getRelationId(IA2_RELATION_DETAILS_FOR, pacc);
402-
VBufStorage_controlFieldNode_t* detailsParentNode = nullptr;
403-
std::optional<std::wstring> detailsRole;
404-
if (detailsId.has_value()) {
405-
parentNode.addAttribute(L"hasDetails", L"true");
406-
detailsParentNode = &parentNode;
407-
auto detailsChildNode = buffer.getControlFieldNodeWithIdentifier(docHandle, detailsId.value());
408-
if (detailsChildNode != nullptr) {
409-
detailsRole = detailsChildNode->getAttribute(L"role");
461+
of the nodes in the relationship will not be in the buffer yet.
462+
It is possible that nodeBeingFilled is both the target and origin of multiple details relations.
463+
*/
464+
// handle case where nodeBeingFilled is the origin of a details relation.
465+
auto idOfDetailsTargets = getAllRelationIdsForRelationType(IA2_RELATION_DETAILS, pacc);
466+
if (0 < idOfDetailsTargets.size()) {
467+
nodeBeingFilled.addAttribute(L"hasDetails", L"true");
468+
for (const auto idOfDetailsTarget : idOfDetailsTargets) {
469+
auto detailsTargetNode = buffer.getControlFieldNodeWithIdentifier(docHandle, idOfDetailsTarget);
470+
if (detailsTargetNode != nullptr) {
471+
const std::wstring roleName = L"role";
472+
auto targetDetailsRole = detailsTargetNode->getAttribute(roleName);
473+
if (targetDetailsRole.has_value()) {
474+
_extendDetailsRolesAttribute(nodeBeingFilled, *targetDetailsRole);
475+
}
476+
else {
477+
/*
478+
This is not an error, the target may not have a role listed in its attributes.
479+
Add "unknown" to the ariaDetailsRoles.
480+
Explanation:
481+
hasDetail=True is not enough to describe the scenario when there multiple details
482+
relations but one has a generic role, EG. a comment and an 'unknown' role.
483+
This would then produce attributes 'hasDetail=True detailsRoles="comment"',
484+
reported as "has comment" since only one role is listed.
485+
Instead prefer 'hasDetail=True detailsRoles="comment, unknown"',
486+
reported as "has comment has details" since both roles are listed.
487+
*/
488+
_extendDetailsRolesAttribute(nodeBeingFilled, L"unknown");
489+
}
490+
}
410491
}
411492
}
412-
if (detailsForId.has_value()) {
413-
detailsParentNode = buffer.getControlFieldNodeWithIdentifier(docHandle, detailsForId.value());
414-
detailsRole = roleAttr;
415-
}
416-
if (detailsParentNode != nullptr && detailsRole.has_value()) {
417-
detailsParentNode->addAttribute(L"detailsRole", detailsRole.value());
493+
// handle case where nodeBeingFilled is the target of a details relation.
494+
auto idOfDetailsOrigins = getAllRelationIdsForRelationType(IA2_RELATION_DETAILS_FOR, pacc);
495+
for(const auto idOfDetailsOrigin : idOfDetailsOrigins){
496+
auto detailsOriginNode = buffer.getControlFieldNodeWithIdentifier(docHandle, idOfDetailsOrigin);
497+
if (detailsOriginNode != nullptr) {
498+
_extendDetailsRolesAttribute(*detailsOriginNode, nodeBeingFilledRole);
499+
}
418500
}
419501
}
420502

‎nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.h‎

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/*
22
This file is a part of the NVDA project.
33
URL: http://www.nvda-project.org/
4-
Copyright 2006-2022 NVDA contributors.
4+
Copyright 2006-2023 NVDA contributors.
55
This program is free software: you can redistribute it and/or modify
66
it under the terms of the GNU General Public License version 2.0, as published by
77
the Free Software Foundation.
@@ -33,12 +33,20 @@ class GeckoVBufBackend_t: public VBufBackend_t {
3333
bool ignoreInteractiveUnlabelledGraphics=false
3434
);
3535

36+
/* Fill the virtual buffer with information required for aria details (annotations) information.
37+
* @note: Expected to be called by fillVBuf
38+
* @param docHandle: The handle of the current document, required to identify target/origin of an annotation relationship.
39+
* @param pacc: The IAccessible for the nodeBeingFilled
40+
* @param buffer: The virtual buffer that is being filled, used to get the other side of the relationship (target/origin)
41+
* @param nodeBeingFilled: The current node being filled. This maybe a target, origin, or neither.
42+
* @param nodeBeingFilledRole: The role of the parentNode, used to set the detailsRole in the origin of the annotation relationship.
43+
*/
3644
void fillVBufAriaDetails(
3745
int docHandle,
3846
CComPtr<IAccessible2> pacc,
3947
VBufStorage_buffer_t& buffer,
40-
VBufStorage_controlFieldNode_t& parentNode,
41-
const std::wstring& roleAttr
48+
VBufStorage_controlFieldNode_t& nodeBeingFilled,
49+
const std::wstring& nodeBeingFilledRole
4250
);
4351

4452
void versionSpecificInit(IAccessible2* pacc);
@@ -47,9 +55,28 @@ class GeckoVBufBackend_t: public VBufBackend_t {
4755

4856
std::wstring toolkitName;
4957

50-
std::optional<int> getRelationId(LPCOLESTR ia2TargetRelation, IAccessible2* pacc2);
58+
/* Get the IAccessible IDs for all relation targets of the specified relation type.
59+
* @param ia2TargetRelation: The type of relation to fetch. Use IA2_RELATION_* constants
60+
from 'include/ia2/api/AccessibleRelation.idl' becomes 'build/<arch>/ia2.h'
61+
* @param pacc2: The element to fetch relations for.
62+
*/
63+
std::vector<int> getAllRelationIdsForRelationType(LPCOLESTR ia2TargetRelation, IAccessible2* pacc2);
64+
65+
/*
66+
*/
5167
std::optional< LabelInfo > getLabelInfo(IAccessible2* pacc2);
52-
CComPtr<IAccessible2> getRelationElement(LPCOLESTR ia2TargetRelation, IAccessible2_2* element);
68+
69+
/* Get relation elements of the type.
70+
* @param ia2TargetRelation: The type of relation to fetch. Use IA2_RELATION_* constants
71+
from 'include/ia2/api/AccessibleRelation.idl' becomes 'build/<arch>/ia2.h'
72+
* @param element: The element to fetch relations for.
73+
* @param numRelations: If supplied, only return up to numRelations. Note: Fetches all by default.
74+
*/
75+
std::vector<CComQIPtr<IAccessible2>> getRelationElementsOfType(
76+
LPCOLESTR ia2TargetRelation,
77+
IAccessible2_2* element,
78+
const std::optional<std::size_t> numRelations = {}
79+
);
5380
CComPtr<IAccessible2> getSelectedItem(IAccessible2* container,
5481
const std::map<std::wstring, std::wstring>& attribs);
5582

‎nvdaHelper/vbufBase/storage.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,6 @@ std::optional<std::wstring> VBufStorage_fieldNode_t::getAttribute(const std::wst
328328
if (foundAttrib != attributes.end()) {
329329
return foundAttrib->second;
330330
}
331-
LOG_ERROR(L"Couldn't find attribute " << name);
332331
return std::nullopt;
333332
}
334333

0 commit comments

Comments
 (0)