(a) It is not obvious what DeviceFormat::DEFAULT means
(b) There are two parameters (each with two states), but only really 2
possible overall states
So
(1) use more useful names
(2) combine the two parameters into one enum
Change-Id: Ic0595b39e032cc9e019b88326389d055b977da00
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/146589
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
The base UnoControl uses WeakAggImplHelper9, so (for better or worse) derives
from XAggregation, but UnoGridControl, UnoRoadmapControl, and UnoTreeControl all
failed to properly implement the XAggregation protocol.
Change-Id: Ia1676782e616216f645dc86ba978c2f5887dc508
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/145767
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
...as there were no calls to its OAccessibleImplementationAccess::getUnoTunnelId
so no code apparently made use of that XUnoTunnel
Change-Id: I35d392999f1c28e81f7ff4bf395f63642eb17bce
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/145250
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
...after 21e0d8162a "loplugin:unusedfields" had
dropped the base class
OAccessibleImplementationAccess::implGetForeignControlledParent
Change-Id: I753aad41baaa0ac1d1f7eb0a6084993df5a58d5f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/145197
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
(See the upcoming commit introducing that loplugin:unocast on why such
dynamic_casts from UNO types are dangerous.)
Change-Id: I11496cc1d37e89ce8f11991f86c7b60bb1b93106
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144748
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
(See the upcoming commit introducing that loplugin:unocast on why such
dynamic_casts from UNO types are dangerous.)
Change-Id: Ia3bf3f52845884e68094b966ad3bd49ad82688a8
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144749
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
(See the upcoming commit introducing that loplugin:unocast on why such
dynamic_casts from UNO types are dangerous.)
Change-Id: I7f755ce1483c5e2eca3066d12d90c5916d2f9087
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144751
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
(See the upcoming commit introducing that loplugin:unocast on why such
dynamic_casts from UNO types are dangerous.)
Change-Id: I33fc079e2128c678ecc36567c2c5ed75481694c6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144752
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
(See the upcoming commit introducing that loplugin:unocast on why such
dynamic_casts from UNO types are dangerous.)
Change-Id: I90754509513cafd04fa560d76a669a47d587f353
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144750
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
to try and untangle the dependency mess a little.
This means
(*) tunneling a piece of information from VCLXWindow via
the XWindowPeer::getProperty method, so
we do not need to directly call VCLXWindow.
(*) moving some formpdf code from framework "down" to vcl,
where the drawinglayer code can use it
Change-Id: I2c1ff3e34941b3147e66cf0a2d0fdb13293e3aa2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144341
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
(See the upcoming commit introducing that loplugin:unocast on why such
dynamic_casts from UNO types are dangerous.)
Change-Id: Ic151fd657cb780291009ee4c9db51a35dfa7f357
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144335
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
This patch fixes tdf#152012 which caused an assertion failure on opening
date picker field in a DOCX file
The assertion was:
include/o3tl/span.hxx:83: constexpr o3tl::span<T>::value_type& o3tl::
span<T>::operator[](o3tl::span<T>::size_type) const [with T = const
int; o3tl::span<T>::reference = const int&; o3tl::span<T>::size_type
= long unsigned int]: Assertion `pos < size()' failed.
And the backtrace was:
1 __pthread_kill_implementation pthread_kill.c:44
2 __pthread_kill_internal pthread_kill.c:78
3 __GI___pthread_kill pthread_kill.c:89
4 __GI_raise raise.c:26
5 __GI_abort abort.c:79
6 __assert_fail_base assert.c:92
7 __GI___assert_fail assert.c:101
8 o3tl::span<int const>::operator[] span.hxx:83
9 OutputDevice::ImplLayout text.cxx:1396
10 OutputDevice::DrawTextArray text.cxx:948
11 Calendar::ImplDraw calendar.cxx:71
12 Calendar::Paint calendar.cxx:1133
The problem was caused by an out of bound access to a vector of integers
which was created for rendering calendar header consisting of the first
letters of 7 days of week, when you clicked on the down arrow on the
date field.
The function OutputDevice::DrawTextArray() takes an 'rStr' string to
draw, and 'pDXAry' array for the exact position of the the individual
characters. It also takes 'nIndex' as the first index, and 'nLen' as the
length of the array. 'nLen' has the default value of -1. In this case,
the length is calculated from the size of the string passed to the
function. This works well if the one who uses the function makes sure
that the size of the array and the length of string are equal.
Previously, for the 7 days of the week, a 7 letter string "smtwtfs"
(depending on the week start day this can be different, but length is
always 7) was sent to this method without providing the length, thus
the string length: 7 was used. In this case, positions of the letters
were calculated and used from other array named mnDayOfWeekAry[7].
mnDayOfWeekAry[k+1] was used as the position of letter k (k=0..5).
In this case, there was 7 letters for 7 days, and only 6 positions
provided by the array. This caused assertion failure in span.hxx:83
when trying to accesss mnDayOfWeekAry[7] via o3tl::span<T>::operator[].
Value of mnDayOfWeekAry[0] was used in other calculations, therefore to
fix this problem, mnDayOfWeekAry was extended from 7 to 8, and the last
position was set to the end of drawing rectangle.
The other thing that is done in this patch to avoid this problem in the
future is removing the default value from the function prototype, so
that the use should always be done by providing the length of array and
starting index. After removing these defaults, it became necessary to
provide empty arrays for 'pKashidaAry' which provides the kashida
positions, if needed.
With this fix in place, the assertion failure no longer happens.
A UI test is added to make sure the crash will not happen again. The
test can be run by invoking:
cd sw && make -srj1 UITest_writer_tests5 \
UITEST_TEST_NAME="DateFormFieldPropertiesDialog.dateFormFieldDialog.test_date_picker_drop_down" \
SAL_USE_VCLPLUGIN=gen
Change-Id: I347afb358fbc4956524f7f0a0abc3a221bf42992
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/142642
Tested-by: Jenkins
Reviewed-by: خالد حسني <khaled@aliftype.com>
i.e. ignore system theme so we get the same results on export to pdf
regardless of the theme (esp dark) and don't follow the system theme
when hosted with a writer/calc/impress document (do continue to use
system theme for StarBasic dialogs as seen in BasicIDE)
Didn't reuse 'NativeWidgetLook' for this because is currently defaults
off, while we currently do use the colors derived from the system theme
even when this is off, its really the NWF flag to render using the
platform theming engine
Change-Id: I816d7ebaf793e5eac7bd937d44c1db0371145199
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/140942
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
When a `vcl::Window` becomes visible, `Window::ImplSetReallyVisible`
calls the registered event listeners with a
`VclEventId::WindowShow` event. Likewise, a
`VclEventId::WindowHide` events is sent in
`Window::ImplResetReallyVisible` when the window
is no longer visible.
Handle that event in `VCLXAccessibleComponent` by
sending a state change event for the SHOWING state,
so assistive technology gets notified about this.
(Similar handling can already be found e.g. in
`AccessibleTabBar::ProcessWindowEvent` or
`AccessibleTabBarPageList::ProcessWindowEvent`.)
While doing so in `VCLXAccessibleComponent::ProcessWindowEvent`
for the object itself would generally seem like a more straightforward
and conceptually nicer approach, this would have the problem
that the event wouldn't get propagated to the platform-specific
a11y integration layer (like winaccessibility) for the
`VclEventId::WindowShow` case, since the a11y event
listeners are registered and unregistered as a response to the CHILD
event (at least for winaccessibility and gtk3, qt6 doesn't do that
(yet?)), and if the accessible event listener is not (yet) registered,
the event is simply ignored.
Since the CHILD event is sent in
`VCLXAccessibleComponent::ProcessChildWindowEvent` and that gets
called on the parent *after* `VCLXAccessibleComponent::ProcessWindowEvent`
gets called for the object that became shown/hidden
(s. `Window::CallEventListeners`), also send the state change
event for the SHOWING state of the child from there, so the
proper order can be made sure.
The reverse order (first the state change event for the SHOWING
state, then the CHILD event which results in removal of the a11y
event listeners) is used for the case where the window gets
hidden.
In combination with
Change-Id Ifcf9304883e2e824ea1b7998d7767e474b87c8b6
("tdf#119788 tdf#117173 add atk notification role") and
commit 155e8b1683
("tdf#117173: qt a11y: Forward changes to SHOWING state"),
this makes Orca announce the label with notification role
in the Search and Replace dialog for the qt6
VCL plugin as well.
The gtk3 case already works with just
Change-Id Ifcf9304883e2e824ea1b7998d7767e474b87c8b6
("tdf#119788 tdf#117173 add atk notification role") in place,
because that one uses native gtk widgets in the Search and Replace
dialog, and the Gtk library then takes care of sending the
object:state-changed:showing event when the label
with notification role gets shown.
Side note: There are also comments that suggest to rework
the a11y event handling for the show/hide changes
more fundamentally, e.g. this comment in
`Window::ImplSetReallyVisible`:
// the SHOW/HIDE events serve as indicators to send child creation/destroy events to the access bridge.
// For this, the data member of the event must not be NULL.
// Previously, we did this in Window::Show, but there some events got lost in certain situations. Now
// we're doing it when the visibility really changes
if( bBecameReallyVisible && ImplIsAccessibleCandidate() )
CallEventListeners( VclEventId::WindowShow, this );
// TODO. It's kind of a hack that we're re-using the VclEventId::WindowShow. Normally, we should
// introduce another event which explicitly triggers the Accessibility implementations.
Similar ones can be found in `Window::ImplResetReallyVisible`
and `Window::Show`.
Change-Id: Id62b3942dc17c3a1ed6a08d23438406e5a19c39d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/139813
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
The highlight and highlighttext colors can be set for some
controls. So as example a selected item in a listbox can now
be paint with anothers colors then the standard blue. Controls
are: listbox, combobox, edit field and some special edit
fields like date, currency and others.
Change-Id: Iace2dd9a1a61abb7819b6c81eb0b8030912db32b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/136691
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
Tested-by: Jenkins
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>
Jusify() normalizes the rectangle so both the Width and Height are
positive, without changing the location of the rectangle. It ensures
that the x and y coordinates will be moved to the top left of the
rectangle.
The name is strange, so renaming Justify() to Normalize().
Change-Id: Idbf163e65e52a798e38f785b8961b8042cf0cf2a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/137379
Tested-by: Jenkins
Reviewed-by: Tomaž Vajngerl <quikee@gmail.com>
...so that its TOOLS_WARN_EXCEPTION can be used in
comphelper/source/misc/logging.cxx in a follow-up commit. (And while at it,
rename from diangose_ex.h to the more appropriate diagnose_ex.hxx. The
comphelper module is sufficiently low-level for this immediate use case, so use
that at least for now; o3tl might be even more suitable but doesn't have a
Library until now. Also, for the immediate use case it would have sufficed to
only break DbgGetCaughtException, exceptionToString, TOOLS_WARN_EXCEPTION,
TOOLS_WARN_EXCEPTION_IF, and TOOLS_INFO_EXCEPTION out of
include/tools/diagnose_ex.h into an additional new
include/comphelper/diagnose_ex.hxx, but its probably easier overall to just move
the complete include file as is.)
Change-Id: I9f3222d4ccf1a9ac29d7eb9ba1530d53e2affaee
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/138451
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>