... that created some local language dialect.
Mostly an automatic rewrite, with clang-format applied where
it is unavoidable.
Change-Id: Ibc24d00f6e80000320fa9bfb35ee16add0f714cc
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/131961
Tested-by: Mike Kaganski <mike.kaganski@collabora.com>
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
It is simply useless, and in vast majority of cases, it simply
nullified variables immediately before assigning them new values.
Change-Id: Ibdad31229069d3cc7000624d5fca372472e8b59c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/131955
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
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>
Use a range-based for loop, `OUString::isEmpty`
instead of checking the length and
combine the two conditions into one.
Change-Id: Iddc75369b1600f2f540f7ec4a6f8ed0b8462fd93
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/130841
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
And use const reference to css::style::TabStop instead of
creating a copy.
Change-Id: I4092cbefcfbdaa6a6697974b7f40242e26cc6aad
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/130758
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
This is similar to
Change-Id: If4b295548468067c65afcd1a216d29cad9e0c213
("wina11y: Unify how AccessibleRole is referred to here"),
just for `AccessibleStateType`.
Change-Id: I5e99fad42dab87e0e303e5a08c610f6af77641ed
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/130700
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Using
using namespace com::sun:⭐:accessibility::AccessibleRole;
and then
case /*AccessibleRole::*/DIALOG
looks a bit odd. Drop the `using` directive and
consistently use `AccessibleRole::<ROLE>` across
this source file when referring to `AccessibleRole`s.
Change-Id: If4b295548468067c65afcd1a216d29cad9e0c213
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/130699
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Also drop the previous comments for the constants that add
little value and are partially outdated.
See the actual implementation to see how values are mapped
to send out actual MSAA events, e.g. the previous comment
// CARET_CHANGED --> EVENT_OBJECT_LOCATIONCHANGED
was no longer correct by now, since that is by now mapped to
`IA2_EVENT_TEXT_CARET_MOVED` in `AccObjectWinManager::NotifyAccEvent`.
Change-Id: I38111ccb60c06f73ce53b18a6ca32e13dbd629b8
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/130697
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
`ACC_DEBUG` isn't defined anywhere and the
`pFile` variable/member used inside of the `ifedf`ed block
doesn't exist either.
Change-Id: I0d04ca71dbc9a731a6673fad9b69c1229c472d16
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/130696
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Quoting MSAA doc about implementing child IDs [1]:
> # How Servers Implement Child IDs
>
> Server developers can assign child IDs to both simple elements and
> accessible objects. However, the recommended approach is to support the
> standard Component Object Model (COM) interface IEnumVARIANT in every
> accessible object that has children.
>
> If you implement IEnumVARIANT, you must:
>
> * Enumerate all children, both simple elements and accessible objects.
> Provide child IDs for all simple elements and provide the IDispatch to
> each accessible object.
> * For accessible objects, set the vt member of the VARIANT to
> VT_DISPATCH. The pdispVal member must contain a pointer to the IDispatch
> interface. Note that the VARIANT is allocated and freed by the client.
> * For simple elements, the child ID is any 32-bit positive integer.
> Note that zero and negative integers are reserved by Microsoft Active
> Accessibility. Set the VARIANT structure vt member to VT_I4 and the lVal
> member to the child ID.
>
> If you do not support IEnumVARIANT, you must assign child IDs and
> number the children in each object sequentially starting with one.
So far, LibreOffice was returning negative "child IDs" instead of pointers
to accessible objects, which were not conformant to the MSAA
specification and not accepted by NVDA as valid child IDs
(s.a. discussion on the first version of my related NVDA pull request
to fix the announcement of a single selected cell in Calc, [2]).
Adapt that to return pointers to accessible objects and
drop the now unused 'CMAccessible::Get_XAccChildID'.
[1] https://docs.microsoft.com/en-us/windows/win32/winauto/how-servers-implement-child-ids
[2] https://github.com/nvaccess/nvda/pull/13277
Change-Id: I52a6f637adf334dee66627e6992451e6d81a7c9a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/129201
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
'IMAccessible' derives from 'IAccessible2', which in turn
derives from 'IAccessible', so there's no need for any
explicit cast from 'IMAccessible*' to 'IAccessible*'.
Change-Id: Id5ad2d778f040364a1a056916a0ad1fe306707e8
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/129200
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
While 'MyClass:: MyMethod' (with a space in it) is valid in C++,
it's unusual and may break a naive plain-text search for
a specific class member definition.
Change-Id: I8c9779865715a0b4f1a66d3e35b874f2cec19193
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128016
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Adapt 'MAccessible::get_accDescription' to directly
retrieve the accessible description on demand via the
corresponding XAccessible, rather than keeping
track of it in a class member.
This simplifies the handling and makes it
unnecessary to "manually" update the description
on 'accessibility::AccessibleEventId::DESCRIPTION_CHANGED'
events, since the new value will
be queried next time it is needed anyway.
This also fixes the problem that a significant
amount of time was spent generating accessible
descriptions for all newly inserted a11y objects when
entering values into Calc cells with the NVDA
screen reader in use, resulting in several
seconds of delay.
Querying up-to-date values from the underlying
UNO interfaces on demand instead of doing extra
manual bookkeeping in the winaccessibility code
may be possible for more a11y attributes in addition
to the accessible description handled in this commit,
but each one will have to be looked at separately.
Change-Id: I57f4c523ca8b10afad3f9c347c8ff5e9420ad968
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128006
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
The accessible description and the accessibile *action*
description are two different things.
'AccObject::UpdateActionDesc' was (IMHO somewhat confusingly)
handling both.
Drop the handling of the accessible description from there
and call the already existing 'AccObject::UpdateDescription'
at the only call site of 'AccObject::UpdateActionDesc' in
addition, which does exactly the same thing.
Change-Id: I220df676f128a5db69ab4cebe7772d92f6b9b0ff
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128005
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Use a separate local variable for the accessible
*action* description here, which is independent of
the accessible description handled above.
While at it, use '!OUString::isEmpty' instead of
'OUString::getLength() > 0'.
Change-Id: I146d8603ed3decde68f8ce716ac13c04b0263040
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128004
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
...instead of by listing the content somewhat redundantly in the Rdb_*.mk
files, to avoid duplication of logic for components that are only built
conditionally (and thus should only be included conditionally in the
corresponding Rdb). To achieve that, add an "rdb" parameter to
gb_ComponentTarget_ComponentTarget (and to the gb_*_set_componentfile macros
that internally call gb_ComponentTarget_ComponentTarget), which is used to make
the appropriate gb_Rdb_add_component call internally from within
gb_ComponentTarget_ComponentTarget. (As a special case,
gb_CppunitTest_set_componentfile shall not call gb_Rdb_add_component, as that
has already been done by the corresponding gb_Library_set_componentfile call, so
allow the gb_ComponentTarget_ComponentTarget "rdb" parameter to be empty to
support that special case.)
Most Rdb_*.mk files are thus mostly empty now. One exception is
i18npool/Rdb_saxparser.mk, which duplicates some of the Rdb_services content as
needed during the build in CustomTarget_i18npool/localedata.
1c9a40299d "gbuild: create services.rdb from built
components" had already tried to do something similar (in addition to other
things) under a new --enable-services-rdb-from-build option. However, that
approach had four drawbacks that this approach here addresses (and which thus
partly reverts 1c9a40299d):
1 Rdb_services shall not contain the component files of all libraries that are
built. While that commit filtered out the component files that go into
Rdb_ure/services (ure/Rdb_ure.mk), it failed to filter out the component files
that go into others like Rdb_postgresql-sdbc
(connectivity/Rdb_postgresql-sdbc.mk).
2 The code added by that commit to Makefile.gbuild codified the knowledge that
there is an Rdb_services, which is brittle.
3 The code added by that commit to solenv/gbuild/Rdb.mk codified the knowledge
(for gb_Rdb__URECOMPONENTS) that there is an Rdb_ure/services, which is brittle.
4 Introducing an --enable-services-rdb-from-build option needlessly provided
two different ways how the content of Rdb_services is assembled.
The changes done here would leave --enable-services-rdb-from-build as a
misnomer, as it no longer controls how Rdb_services is assembled. I thus
renamed it to --enable-customtarget-components, as that is apparently what it
still does now.
Change-Id: Ia5e8df4b640146c77421fcec6daa11a9cd260265
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/126577
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
...which defines NULL as a plain 0 integer literal instead of the GNU __null
extension, so clang-cl's -Wnull-conversion cannot kick in. These findings are
from an experimental build done with clang-cl and a modified
> --- a/clang/lib/Headers/stddef.h
> +++ b/clang/lib/Headers/stddef.h
> @@ -83,6 +83,10 @@ typedef __WCHAR_TYPE__ wchar_t;
> # if !defined(__MINGW32__) && !defined(_MSC_VER)
> # define NULL __null
> # else
> -# define NULL 0
> +# if __cplusplus >= 201103L
> +# define NULL nullptr
> +# else
> +# define NULL 0
> +# endif
> # endif
> #else
> # define NULL ((void*)0)
However, that build also ran into lots of places where 3rd-party code in
external/ and Windows system headers caused issues when NULL is nullptr (which
I worked around with various hacky patches for that build), so this is
unfortunately not something that can easily be enabled generally.
Change-Id: I10674464498a9bc63578d9e6cc32ddde23ab4f30
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/124419
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
... and 'AccObject::NotifyDestroy', respectively.
The only calling site calls it with 'true' as parameter
(and 'CMAccessible::NotifyDestroy' was calling
'm_xAccessible.clear()' regardless of what value the
passed param had anyway).
Change-Id: I08a24f79b50ec327ae461bd44ee6460a2d8145f7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122343
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
The getter, 'AccObject::ifShouldDestroy',
was unused, so drop it.
The only place where the 'm_bShouldDestroy' member
was actually used was in 'AccObject::NotifyDestroy',
where it was assigned a value from a method param that
was then passed further right away, so there's no need to
store the value at all.
Change-Id: Id5688e9742892e345b143a8eebeed5099696a0f2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122342
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Create a clone and reset the IEnumVariant returned
in 'CMAccessible::get_accSelection'
when multiple children are selected.
It seems reasonable to assume that an 'IEnumVARIANT'
retrieved by calling the 'IAccessible::get_accSelection' [1]
method to get access to the selected children will
start enumerating selected children at the beginning,
not where enumeration left off with any previously
retrieved 'IEnumVARIANT'.
I came across this because a pending NVDA pull request
to implement the announcement of multiple selected Calc cells
mentions that "accSelection is broken in LibreOffice" [2].
NVDA uses 'IAccessible::get_accSelection' and then
'IEnumVARIANT::Next' to get the amount of currently
selected cells.
However, with the same underlying 'CMAccessible' object
being used in subsequent attempts to retrieve the count
of selected children and its 'm_lCurrent' member not
being reset in between, this effectively meant that children
considered when retrieving the selection the previous time
were considered as "already having been taken into account"
when (not) looping over the children to return in
'CEnumVariant::Next'.
[1] https://docs.microsoft.com/en-us/windows/win32/api/oleacc/nf-oleacc-iaccessible-get_accselection
[2] b9f7ee1c2a
Change-Id: I8e6b8bffaab010a0cfa7db8d43807e7520673d45
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122268
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
... by changing the first param for
AccObjectManagerAgent::SaveTopWindowHandle
to HWND.
Change-Id: Ia94bd95c49a64e63b1be332bd03e83a288707d80
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122204
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
... by changing the corresponding params for
AccObjectManagerAgent::InsertAccObj and
AccObjectManagerAgent::InsertChildrenAccObj
to HWND.
Change-Id: I598bac30d827ae9166bc21cb5e8dff88a5cc99bf
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122203
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
They are defined in WinUser.h, so there's no
need to duplicate that.
Change-Id: I1ab4255a8964d7822eface027d3cef7dcd6d29eb
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122201
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
When handling a SELECTION_CHANGED_REMOVE event,
AccDescendantManagerEventListener previously emitted a corresponding
EVENT_OBJECT_SELECTIONREMOVE MSAA event with the removed child's ID
as a parameter, then deleted that child's accessibility object.
This resulted in the accessibility object no longer being available
if accessibility tools queried for the object just received in
the EVENT_OBJECT_SELECTIONREMOVE event.
As a consequence, no 'event_selectionRemove' would be triggered
for any table cell object on NVDA side, so an updated selection
would not be announced for that case.
To keep child objects available for such cases, don't delete the
corresponding object in the handler for the SELECTION_CHANGED_REMOVE
handler, but remember it for deletion and only do the actual deletion
once a subsequent different event is handled.
With this in place, the announcement for multiple selected cells in
Calc generally works with the NVDA screen reader and a pending
NVDA pull request [1] by Leonard de Ruijter in place
(s.a. discussion in NVDA issue [2] for more background
and further information).
(There are still cases where events in response
to selection changes are missing, as described e.g. in
tdf#118508 and tdf#118748, but those are not winaccessibility-specific
and it's the same when using the gtk3 VCL plugin with the Orca screen
reader on Linux.)
[1] https://github.com/nvaccess/nvda/pull/12849
[2] https://github.com/nvaccess/nvda/issues/9310
Change-Id: I7a46060c501ba9b8288e7c127121a535c60ce2bc
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122169
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
instead of including the header, as suggested by
Mike Kaganski in the Gerrit change for
commit 6bf771f0a5
Author: Michael Weghorn <m.weghorn@posteo.de>
Date: Thu Sep 9 11:36:57 2021 +0100
wina11y: Use XAccessible* instead of void*
Change-Id: If52de3a71af63697815e4ec6a19f9139d5a6973d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122085
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
(The header had actually been included previously, so
the above commit had added it to the same header a second time...)
Change-Id: Id6a6c5e2426bf07e41f29e1473ec1694970272c5
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122132
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Add a new class 'AccTableCell' that implements the
IAccessibleTableCell interface from the IAccessible2 spec
and add what's needed to expose it to accessibility tools
via COM.
Since there's no specific XInterface for table cells
that an XAccessible's context could be queried for,
make use of the fact that a table cell's
parent is a table, i.e. its accessible context implements
XAccessibleTable.
AccTableCell keeps a reference to that table and remembers the
cell's index in the parent to retrieve information
on the cell from there.
This addresses the
> At least for LibreOffice Table Cells don't implement
> IAccessibleTableCell, and therefore there's no way to get the row and
> column span. LibreOffice itself also does not expose the merged state in
> the accessible name of the cell.
comment from [1] (which is the NVDA counterpart for LO's
tdf#124832) and may also help for tdf#100086,
though more work will be needed on LibreOffice and/or NVDA
side for both issues.
[1] https://github.com/nvaccess/nvda/issues/9310
Change-Id: I0f53212d14ee17c760b9e6c91be2154a1b25d862
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/121821
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Extend 'CAccTable' to implement the 'IAccessibleTable2'
interface in addition to the (deprecated)
'IAccessibleTable' interface from the IAccessible2 spec.
'IAccessibleTable2::get_cellAt' and
'IAccessibleTable2:get_nSelectedCells' are basically
the same as 'IAccessibleTable::get_accessibleAt' and
'IAccessibleTable::get_nSelectedChildren' under
new names.
'IAccessibleTable2::get_selectedRows' and
'IAccessibleTable2::getSelectedColumns' are
essentially the same as their 'IAccessibleTable'
counterparts, except that they have the first
param removed (which is ignored in the
IAccessibleTable version anyway).
'IAccessibleTable2::get_selectedCells' is similar
to 'IAccessibleTable::get_selectedChildren', but
returns an array of references to selected cells,
while the latter just returns an array of their
indices.
Note: Just having the IAccessibleTable2 interface,
but not the IAccessibleTableCell one implemented makes
the experience when using the NVDA screen reader
temporarily worse, e.g. it now only says "selected" instead of
the name of the currently focused cell in Calc.
Implementation of IAccessibleTableCell is added
in an upcoming commit
(Change-Id: I0f53212d14ee17c760b9e6c91be2154a1b25d862,
"tdf#100086 tdf#124832 wina11y: Implement IAccessibleTableCell")
and makes NVDA announce the name of the cell again.
Change-Id: I75346efc3b6e79d5ebf5e1954e9c516244efb887
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/121820
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
