wina11y: Slightly clean up WinResIDGenerator

Changes:

* drop the optional `maxNum` param for the
  constructor, it's never set to anything
  different than the default value
* store negative number in member (renamed
  from `max` to `m_nMin`) instead of storing
  positive number and inverting that when returning
  it in `ResIDGenerator::GenerateNewResID`
* Use -1 as the first resource ID instead of -2.

Also, add a comment that negative child IDs
are used because it's common to use such ones
to indicate unique resource IDs in
IAccessible2.

Quoting James Teh's comment on an NVDA
pull request of mine [1]:

> The use of negative child ids doesn't fit well into the IAccessible
> spec, but it has been done by IAccessible2 for a very long time and
> should be considered standard for all intents and purposes. A negative
> child id should be treated as a unique id, while a positive child id
> should be treated as a child index.
>
> That said, as noted in #13277 (comment), IAccessible2 elements are
> always full IAccessible objects, not "simple elements". Thus, anything
> that returns an accessible (including accSelection) really should return
> an object pointer. In the case of accSelection, this means VT_DISPATCH
> for a single selection or VT_UNKNOWN and iEnumVARIANT (with VT_DISPATCH
> elements) for multiple selection.
>
> In short, NVDA supporting negative child ids returned from accSelection
> isn't necessarily "wrong", but ideally, LibreOffice would be fixed to
> return full objects.

The latter (returning full objects) has already been addressed in

    commit 00c0ee8cf0
    Author: Michael Weghorn <m.weghorn@posteo.de>
    Date:   Mon Jan 31 07:41:14 2022 +0000

        tdf#147083 wina11y: Return a11y object instead of child ID

but a unique ID is still relevant, s. e.g. the documentation
of `IAccessible2::uniqueID` [2], which also mentions potential
ways to implement unique IDs:

> One means of implementing this would be to create a factory with a 32
> bit number generator and a reuse pool. The number generator would emit
> numbers starting at 1. Each time an object's life cycle ended, its
> number would be saved into a reuse pool. The number generator would be
> used whenever the reuse pool was empty.
>
> Another way to create a unique ID is to generate it from a pointer
> value, e.g. an object's address. That would be unique because no two
> active objects can use the same allocated memory space.

The first approach is what the LO implementation currently does,
except that negative numbers are used.

[1] https://github.com/nvaccess/nvda/pull/13277#issuecomment-1024622871
[2] https://accessibility.linuxfoundation.org/a11yspecs/ia2/docs/html/interface_i_accessible2.html#aac1342376cf195a6a6e9ee3b7e30ae3b

Change-Id: I6c0a6c4a4e3a69396205fe2d69cd66af6525a273
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/131927
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
This commit is contained in:
Michael Weghorn 2022-03-22 09:06:26 +01:00
parent 66d1ea5915
commit 6c5257b680
2 changed files with 9 additions and 7 deletions

View file

@ -19,20 +19,22 @@
#pragma once
#define PRIMARY_RESID 0x00000001
#include <deque>
//ResID i.e. MSAA child ID,
//this class is responsible for generating a child ID
//
// In IAccessible2, negative child IDs are commonly used to
// indicate unique resource IDs.
class ResIDGenerator
{
private:
long max;
long m_nMin;
std::deque<long> subList;
public:
ResIDGenerator(long maxNum = PRIMARY_RESID)
: max(maxNum)
ResIDGenerator()
: m_nMin(-1)
{
}
~ResIDGenerator();

View file

@ -25,7 +25,7 @@ ResIDGenerator::~ResIDGenerator() {}
/**
* SubList stores those IDs that were ever generated and deleted, the method
* return the ID from subList first if subList is not empty,else return ++max.
* return the ID from subList first if subList is not empty, else return m_nMin--.
* Add the obsolete IDs by calling SetSub method
*
* @param
@ -39,8 +39,8 @@ long ResIDGenerator::GenerateNewResID()
subList.pop_front();
return nRes;
}
assert(max < LONG_MAX);
return -(++max);
assert(m_nMin > LONG_MIN);
return m_nMin--;
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */