No need for an `else branch` as the `if` branch throws
an exception.
Also stop intermediate assignment to local variable
and return the result directly instead.
Change-Id: Ib442f72510302acffb0336e56df3efd6d93c4d83
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/210609
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
OAccessible returns itself in its XAccessible::getAccessibleContext
implementation and implements the XAccessibleComponent interface itself,
so there is no need to call the method and query for that interface.
Change-Id: I232d86a3c972a8313a6dc62a1b6c799c6b1a9d81
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/210603
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Switch AccessibleChartView::m_xParent from a weak
reference of the abstract XAccessible interface to
a weak reference of the more concrete OAccessible class.
Change-Id: Iba9f410979c079735b6c41723206b2ac76c1dc1b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/210602
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Use the more concrete OAccessible instead of the
abstract XAccessible UNO interface.
Change-Id: I18fd3378abe7c6f81a0244f7901eb0bedc9f0d32
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/210381
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
The helper class used to implement less functionality
in the past and was used less than it is now.
But by now it is the central helper/base class used
by most a11y classes.
Since
commit 22049f633b
Author: Michael Weghorn <m.weghorn@posteo.de>
Date: Mon May 26 12:24:52 2025 +0200
vcl a11y: Let OAccessibleComponentHelper implement XAccessible
it also implements the XAccessible interface.
That will allow to switch parameters and class members
from the abstract XAccessible interface to using this
more concrete C++ base class in various places.
Rename the class to OAccessible to account for the
new responsibilities, and adjust its doc accordingly.
Renaming source and header file will be done in a
separate commit to simplify (git) keeping track.
Change-Id: I33210b47cda5f71820a3c0fa657e4c3548d45732
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/186952
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
By now, in particular after
Change-Id: I62da0040e6c98636681c4f05dbd0b796dae5b928
Author: Michael Weghorn <m.weghorn@posteo.de>
Date: Mon May 26 09:35:07 2025 +0200
vcl a11y: Let VCLXAccessibleComponent implement XAccessible
, all OAccessibleComponentHelper subclasses are
implementing the XAccessible interface and returning
`this` in their XAccessible::getAccessibleContext implementation.
Unify and deduplicate that by moving the implementation to the
OAccessibleComponentHelper base class.
Locking specifics (whether a mutex was locked and which one)
and whether a check whether the accessible hadn't been
disposed yet was done varied across the different implementations.
The new OAccessibleComponentHelper::getAccessibleContext
unifies this to use OExternalLockGuard, i.e. it both locks
the mutex and throws an exception if the instance is no
longer alive.
Should this commit result in any issues, it's likely because
there is a preexisting locking issue/inconsistency or an object
has already been disposed and that needs to be addressed.
(The easiest approach in case of a disposed object would be
to simply catch + ignore the exception in the caller, but
a more thorough investigation would likely be helpful.)
Change-Id: Ib56a9ddb1c36356943c2dfd7a5705351ee43e9d9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/185849
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Tested-by: Jenkins
Stop implementing the XServiceInfo interface in chart2
a11y implementations as the info provided by that interface
isn't needed anywhere, see also
Change-Id: Id306320710a1718ff10536127427f3a5daf68559
Author: Michael Weghorn <m.weghorn@posteo.de>
Date: Fri Jun 20 11:14:19 2025 +0200
svtools ruler a11y: No longer implement XServiceInfo
for more background.
Related to that, drop the unpublished AccessibleChartElement
service.
A search in Google and on Ask [1] didn't give
any results that would suggest this unpublished service
could be relevant for third party code. (Instances.
couldn't be created by external code anyway.)
[1] https://ask.libreoffice.org/
Change-Id: I97056996bc433940c262e9b1060eb37a24a0b00e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/186758
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
We used to have two duplicating sets: smart pointer-like VclPtr::reset,
and Reference-style VclPtr::set / VclPtr::clear. There is no need in
this duplication. Further, in ScopedVclPtr, reset is hidden / removed
to avoid skipping disposeAndClear; but the twin functions are kept
accessible, possibly as an overlook.
This change drops Reference-style VclPtr::set / VclPtr::clear, and
unifies on VclPtr::reset, which looks more consistent with the class
nature (it is closer to shared_ptr, than to uno::Reference). Doing
that uncovered one place, where a ScopedVclPtr was used incorrectly:
clear() was used for NotebookbarPopup::m_pParent in NotebookbarPopup
dtor. I assume then, that the correct class needed here is VclPtr.
Change-Id: I0746075c58d793fabdce1bc357afea8ee455cd8a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/185553
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Tested-by: Jenkins
... and drop the additional check.
The only caller AccessibleChartElement::InitTextEdit
passes (always non-null) `this`.
Change-Id: I218dc224eb6bad827fce66db02706bb555831a8d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/184786
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Drop overrides of the XAccessibleExtendedComponent
which just return an empty string.
That is now already implemented in the base class
implementations in OAccessibleComponentHelper, see
Change-Id: I05547b12806a6a90b166edbe196b4bc5a27344be
Author: Michael Weghorn <m.weghorn@posteo.de>
Date: Mon Mar 31 13:49:20 2025 +0200
a11y: Merge OAccessible{,Extended}ComponentHelper, add default impl
Change-Id: Ibc3fe295eaa2dc5443ba4f8fa25a81451b7b3765
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/183553
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Tested-by: Jenkins
Merge the two classes, now that OAccessibleComponentHelper
is the only OCommonAccessibleComponent subclass after
Change-Id: I05547b12806a6a90b166edbe196b4bc5a27344be
Author: Michael Weghorn <m.weghorn@posteo.de>
Date: Mon Mar 31 13:49:20 2025 +0200
a11y: Merge OAccessible{,Extended}ComponentHelper, add default impl
Change-Id: Ic7a36c40066f307f8df29dc4dd45ed3ee966bcb2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/183552
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Tested-by: Jenkins
Merge OAccessibleExtendedComponentHelper and
OAccessibleComponentHelper by letting
OAccessibleComponentHelper already implement the
XAccessibleExtendedComponent interface.
Add default implementations for the two
XAccessibleExtendedComponent methods
(XAccessibleExtendedComponent::getToolTipText
and XAccessibleExtendedComponent::getTitledBorderText)
that simply return an empty string.
Replace all uses of XAccessibleExtendedComponent
by OAccessibleComponentHelper.
This simplifies the class hierarchy and prepares
for further simplification in upcoming commits.
Change-Id: I05547b12806a6a90b166edbe196b4bc5a27344be
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/183551
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Use the base class's OCommonAccessibleComponent::NotifyAccessibleEvent
directly instead.
The old/new params are in reverse order
for that one, so adjust callers accordingly.
Change-Id: I8d4ef730a139366c5762836819e20d35cb2ff5a6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/183536
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Tested-by: Jenkins
Let chart's AccessibleBase derive from
OAccessibleExtendedComponentHelper to make use of the
logic already implemented there instead of having to
implement all the XAccessible(Extended)Component
and XAccessibleEventBroadcaster methods manually.
AccessibleBase::getBounds implements what is needed
to implement OAccessibleExtendedComponentHelper::implGetBounds,
so rename the method.
Drop all of the other overrides that are no longer
needed as the implementation is now provided by
the base class.
Adjust subclasses as well to deduplicate logic there, too.
No change in behavior intended or observed initially
when testing this with Accerciser and Orca and the gtk3 and qt6
VCL plugins for a chart inserted in Writer ("Insert" -> "Chart").
Change-Id: I5d29c099dc31d70711f4017e52a0859f22c5aa23
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/183524
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Let the base class AccessibleBase implement
XAccessibleExtendedComponent with default implementations
for the methods that simply return empty strings.
This simplifies the implementation in subclasses, that
now only need to override XAccessibleComponent
and XAccessibleExtendedComponent methods when they
actually want to have a different behavior, not
to avoid ambiguity due to XAccessibleComponent
being a base class of both of their base classes
(XAccessibleExtendedComponent, AccessibleBase).
Change-Id: I2a6a39a721de19178954db898d52a2d7bc01fcb7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/183523
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Switch AccessibleChartView::m_xWindow,
AccessibleElementInfo::m_xWindow and related code
from using the UNO interface
css::uno::WeakReference<css::awt::XWindow> to using
the concrete implementation class vcl::Window/ChartWindow
directly instead of assuming it's a vcl::Window and then
using VCLUnoHelper::GetWindow to get the vcl::Window
from the XWindow in multiple places.
Rename the members accordingly.
Use a VclPtr and clear it in
AccessibleBase::disposing and AccessibleChartView::disposing.
No change in behavior intended or seen in a quick test using
Accerciser with the qt6 VCL plugin with a chart inserted in Writer
using "Insert" -> "Chart".
Change-Id: If98ef041fdd1a8bf68620e0c8b1e81973e7b01f5
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/183522
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
This makes more obvious that this is null. In particular
calling VCLUnoHelper::GetWindow with a Reference that is
known to be empty/null doesn't make much sense.
This has become more obvious with previous commit
Change-Id: I91d4e9cc40473aa204ec7bcada954cf0f765e6ea
Author: Michael Weghorn <m.weghorn@posteo.de>
Date: Mon Mar 31 06:14:27 2025 +0200
chart a11y: Make clearing weak refs more obvious
Change-Id: Ia544001d609ede8f44003ffd4befc3cddccf045f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/183519
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Tested-by: Jenkins
Drop the local vars that are first assigned
the current value of the members, but then
cleared, and then assigned to the members.
Instead, clear the members directly to make more obvious
what's happening.
Change-Id: I91d4e9cc40473aa204ec7bcada954cf0f765e6ea
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/183518
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Tested-by: Jenkins
Don't let AccessibleBase subclass XEventListener as
there's no need to, and AccessibleBase::disposing
wasn't doing anything anyways.
Change-Id: I6fa5e7cb65853e775792c35ce6571104c8c7edcb
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/183515
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
AccessibleBase::BroadcastAccEvent is only called from non-const
methods, so there is no need for it to be const itself.
Drop the const qualifier and the now no longer needed const_cast
of `this` inside the method.
Change-Id: I9e8bb74d761135b1f22525fa79912f4694ca7654
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/182121
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Use an rtl::Reference of the specific AccessibleBase
instead of a generic uno::Reference<XAccessible> and
then still making assumptions that it's actually an
Accessible object in various places.
Change-Id: Iaccccc3e928d4f47b16b4681404b6c6169ac67c5
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/182120
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Tested-by: Jenkins
This method from the unpublished XAccessibleExtendedComponent
interface is not used by any of the a11y platform bridges, and
I don't know of any platform a11y API that would need it.
In order to report character/font attributes, there is the
XAccessibleText interface and its
XAccessiText::getCharacterAttributes method instead, which
actually gets used by the platform a11y bridges.
Therefore, drop this method to simplify code, and also decouple
the accessibility module a bit further from the toolkit
module without having to reorganize code further.
(VCLXFont from the toolkit module currently gets used in
various implementations.)
Change-Id: I06ea3cc5998a13927b3f869877b28f03ac07c89b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/177809
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
With 16k column support in Calc enabled by default in
commit 4c5f8ccf0a
Date: Tue Mar 8 12:44:49 2022 +0100
change default Calc number of columns to 16384 (tdf#50916)
, the number of Calc cells in a spreadsheet is larger than
SAL_MAX_INT32, meaning that a 32-bit a11y child index is no more
enough and using it resulted in integer overflows in
methods handling corresponding Calc cells in the a11y layer.
This e.g. had the effect of the Orca and NVDA screen readers
not announcing focused or selected cells properly when their
a11y child index was out of the 32-bit integer range.
Switch the internal a11y child indices to 64 bit to
be able to handle this properly internally.
Since the platform APIs (at least AT-SPI on Linux and
IAccessible2 on Windows; from what I can see LO's macOS
a11y bridge doesn't directly expose the child index)
are still restricted to 32 bit, larger child indices
still cannot be exposed via the platform APIs.
As a consequence, use of the the IAccessible2 and
AT-SPI methods that use the child index remains
problematic in those cases where the child index
is larger. However, as an alternative to using the
AT-SPI Table interface and the IAccessibleTable/
IAccessibleTable2 interfaces with the child index
to retrieve information about a specific cell,
both AT-SPI and IAccessible2 also provide interfaces
to retrieve that information directly
from the cell object (TableCell interface for AT-SPI,
IAccessibleTableCell for IAccessible2).
Those interfaces are already implemented/exposed
for winaccessibility (s. `CAccTable`) and the
qt5/qt6/kf5 VCL plugins (s. the `QAccessibleTableCellInterface`
methods implemented in `QtAccessibleInterface`).
With the switch to 64-bit internal a11y child indices,
these now behave correctly for cells with a child
index that doesn't fit into 32 bit as well.
NVDA on Windows already uses the IAccessibleTableCell
interface and thus announcing focused cells works fine
with this change in place.
Orca on Linux currently doesn't make use of the AT-SPI
TableCell interface yet, but with a suggested change to
do so [1], announcement of selected cells works
with the qt6 VCL plugin with a current qtbase dev branch
as well - when combined with the suggested changes
to implement support for the AT-SPI TableCell interface
in Qt [2] [3] and the LO change based on that [4] and
a fix for a nullptr dereference [5].
The gtk3 VCL plugin doesn't expose the AT-SPI
TableCell interface yet, but once it does so
(via `AtkTableCell`), it also works with the
suggested Orca change [1] in place.
(Adding that is planned for an upcoming change,
works with a local WIP patch.)
For handling return values that are larger than what
platform APIs support, the following approach has
been chosen for now:
1) When the return value is for the count of
(selected) children, the maximum value N
supported by the platform API is returned.
(This is what `ScAccessibleTableBase::getAccessibleChildCount`
did previously.)
The first N elements can be accessed by their
actual (selection) indices.
2) When the return value is the child/cell index,
-2 is returned for objects whose index is greater
than the maximum value supported by the platform
API.
Using a non-negative value would mean that the
index would refer to *another* actually existing
child. A child index of -1 on the other hand
tends to be interpreted as "child is invalid" or
"object isn't actually a child of its (previous)
parent any more)". For the Orca case, this would
result in objects with a child index of -1
not being announced, as they are considered
"zombies" [6].
What's still somewhat problematic is the case where
more than 2^31 children are *selected*, since access
to those children still happens by the index into
the selection in the platform APIs, and not all
selected items are accessible this way.
(Screen readers usually just retrieve
the first and last element from the selection and
announce those.)
Orca already seems to apply different handling for the
case for fully selected rows and columns, so
"All cells selected" or "Columns ... to ... selected"
is announced just fine even if more than 2^31
cells are selected.
(Side note: While Microsoft User Interface
Automation - UIA - also uses 32-bit indices, it also
has specific methods in the ISelectionProvider2
interface that allow to explicitly retrieve the
first and last selected item,
`ISelectionProvider2::get_FirstSelectedItem` and
`ISelectionProvider2::get_LastSelectedItem`, but
we currently don't support UIA on Windows.)
Bound checks at the beginning of the methods from the
`XAccessibleContext`, `XAccessibleSelection` and
`XAccessibleTable` interfaces that take a child index
(or in helper methods called by those) should generally
already prevent too large indices from being passed to
the methods in the lower layer code that take smaller
integer types. Such bound checking has been
been added in various places where it wasn't present yet.
If there any remaining issues of this
kind that show after this commit, they can probably be
solved in a similar way (s.e.g. the change to
`AccessibleBrowseBox::getAccessibleChild` in this
commit).
A few asserts were also added at
places where my understanding is that values shouldn't
be larger than what is supported by a called method
anyway.
A test case will be added in a following change.
[1] https://gitlab.gnome.org/GNOME/orca/-/merge_requests/131
[2] https://codereview.qt-project.org/c/qt/qtbase/+/428566
[3] https://codereview.qt-project.org/c/qt/qtbase/+/428567
[4] https://gerrit.libreoffice.org/c/core/+/138750
[5] https://codereview.qt-project.org/c/qt/qtbase/+/430157
[6] https://gitlab.gnome.org/GNOME/orca/-/blob/82c8542002e36e0d3d918088d583162d25136143/src/orca/script_utilities.py#L5155
Change-Id: I3af590c988b0e6754fc72545918412f39e8fea07
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/139258
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Michael Weghorn