All of the other services in the textfield namespace can be added to a
Writer document via insertTextContent so it’s quite reasonable to think
that URL should work too. This patch just adds a note to the
documentation to help people discover the HyperLinkURL text property
which works better in a Writer document.
Change-Id: If00336dac8676ded98f02d03c6e67e366ac867b1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/201147
Reviewed-by: Bartosz Kosiorek <gang65@poczta.onet.pl>
Tested-by: Jenkins
It looks like the documentation for bExact was just an accidental copy
and paste of the aName param. This changes it to explain what it does
based on looking at the code for the implementation.
Change-Id: Ia767064c66b2125193c75da34273a6d4e6035bd8
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/201453
Reviewed-by: Neil Roberts <bpeeluk@yahoo.co.uk>
Tested-by: Jenkins
Previously whenever the scope is changed in the accelerator
configuration dialog it would silently forget any pending assignments.
This patch makes it instead store an array of assignments for each
possible scope and then apply them all when FillItemSet is called. The
arrays are lazily created as each scope is accessed.
Change-Id: I0cbff7945f7c11c0498c63314e288f45495529ef
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/200697
Reviewed-by: Neil Roberts <bpeeluk@yahoo.co.uk>
Tested-by: Jenkins
Previously the assignments for each key were stored as user data for
each entry in the key list box. Instead they are now stored in a vector
which maps key indices to a command string. The key indices are the
indices of KEYCODE_ARRAY. The user data in TAccInfo which was attached
to each entry in the list box no longer has any useful information so
the struct has been removed entirely. The ID of an entry in the list box
is now directly a key index instead of storing a pointer to the user
data.
The goal is to make it easier to store the assignments for multiple
scopes in a later patch.
Change-Id: I267da16210e7354834833b7f5277569e1208a46c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/200696
Tested-by: Jenkins
Reviewed-by: Neil Roberts <bpeeluk@yahoo.co.uk>
Previously the accelerator config dialog was storing a boolean in the
data for each entry in the key list to mark whether it is configurable.
The only time a key can become unconfigurable if it is a reserved key
code. This patch makes the accelerator config page instead store a
sorted array of reserved key codes and then whenever we need to know if
a key is configurable we can just quickly look it up in this array with
a binary chop. The main reason to do this is to make it easier to remove
the TAccInfo in a later patch.
Change-Id: Iaac9a5deb07ae5bbf55e784a80b7193dad5e041a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/200694
Tested-by: Jenkins
Reviewed-by: Neil Roberts <bpeeluk@yahoo.co.uk>
A bunch of places in the documentation in the IDL files were starting a
new paragraph with a <p> tag and a line that is more indented than the
previous one. This ends up making a preformatted code section in a
monospace font in the generated HTML docs and the <p> tag is visible.
This patch just moves the lines to the left to have the same indentation
as the previous one. These were found by running the following Python
script on the generated HTML files:
from bs4 import BeautifulSoup
import sys
for filename in sys.argv[1:]:
with open(filename) as fp:
soup = BeautifulSoup(fp, 'html.parser')
for pre in soup.find_all("pre", class_="fragment"):
if pre.string.find("<p>") != -1:
print(f"{filename}:{pre.sourceline}:"
f"{str(pre).split('\n')[0]}")
Change-Id: If351a5be24b21b24a4d7892a9185353e8691feb1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/201134
Tested-by: Jenkins
Reviewed-by: Neil Roberts <bpeeluk@yahoo.co.uk>
Makes it possible to import the name of a singleton into a Python script
as a class and then retrieve it with a getter method. Eg, instead of:
desktop = ctx.getValueByName(
"/singletons/com.sun.star.frame.theDesktop")
you can now do:
from com.sun.star.frame import theDesktop
desktop = theDesktop.get(ctx)
Change-Id: I009bd2e890cde52aa6273e462621157fc1f63661
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/199219
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <stephan.bergmann@collabora.com>
Instead of explicitly calling createInstance we can use the implicit
service constructor on the Toolkit type for a slightly nicer way to
construct it.
Change-Id: Ia06000afc0cde27209966854704ca8dd61375adf
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/199106
Reviewed-by: Stephan Bergmann <stephan.bergmann@collabora.com>
Tested-by: Jenkins
This leverages the existing framework for embindtest to add interfaces
and services to test with. Hopefully that means the tests will run on
the CI without ending up in the final release builds because they are
only enabled with --enable-embindtest-uno.
Change-Id: Idd394e616ad14943cecd60bf3a3e228d93166e34
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/199105
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <stephan.bergmann@collabora.com>
This makes it so that services can be imported in Python with code like
this:
from com.sun.star.resource import StringResourceWithLocation
And then an instance of the service can be constructed using any of its
constructors as a class method like this:
res = StringResourceWithLocation.create(ctx, root_url, True, locale,
dlg, '', None)
This has the advantage that the number of arguments can be checked
properly and a more useful error message can be reported if they are not
correct. Because pyuno knows what types the arguments are supposed to
be, it can also do a better job of coercing them to the right ones. For
example in the example above it is possible to pass None for an
interface argument even though that normally represents the void type.
This idea evolved out of the discussion for tdf#169229.
Change-Id: I9b542178f1c101d58dcbbd241d4ecbc9fe38bc2c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/199104
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <stephan.bergmann@collabora.com>
When a rest argument is used in an IDL, it was previously just
generating a single Any argument for the constructor and then trying to
pass that directly to createInstanceWithArgumentsAndContext. That
doesn’t compile because that method requires an array of Anys for the
arguments parameter. Instead this patch makes it generate “params Any[]
rest” which seems to be the .NET way to do variadic arguments.
I don’t think there are any LibreOffice services using constructors with
rest parameters and presumably no one is using it outside of LO with
.NET because it wouldn’t compile, so hopefully this patch has little
risk. Fixing it enables us to use rest parameters in a future unit test
for pyuno.
Change-Id: I7dbf96f945011db546374ae1d6cf904a5bd262ce
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/199135
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <stephan.bergmann@collabora.com>
Previously, when assigning a shortcut key, there was only a choice
between assigning it in the current module or globally for all of
LibreOffice. This patch adds a third radio button with a dropdown box to
be able to pick a document to define the key in instead. This
functionality is already available in the toolbar and menu pages.
Change-Id: I465dbb84428268f621174d8e52662175a72f44e5
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/197709
Reviewed-by: Heiko Tietze <heiko.tietze@documentfoundation.org>
Tested-by: Jenkins
If one of the entries in the arguments array for callFunction is a void
type it will now be interpreted as a missing argument. This makes it
possible to call calc functions where a missing argument is meaningful
such as the “replacement” argument to REGEX().
This is a minor API change because previously passing VOID would throw
an IllegalArgumentException. It seems unlikely that anyone would have
been relying on that though so I think the change is pretty harmless.
Change-Id: Iaa4500300e9b4b88a7bdfdc153c67c42cd783525
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/200287
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Tested-by: Jenkins
The idea is to check that all of the class names match a singleton
declared in a component somewhere and that they return an object that
implements the advertised interface. Otherwise the generated getter for
the singleton will fail with a runtime exception.
Change-Id: I1352e7392e33e9182962e6279fcb62cf3b58352a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/200056
Reviewed-by: Neil Roberts <bpeeluk@yahoo.co.uk>
Tested-by: Jenkins
The namespace for theServiceDocumenter singleton doesn’t match the name
of the component declared in utl.component so if you tried to use the
automatically generated getter in C++ or the getter from BASIC then it
wouldn’t work. This patch adds a second singleton name to the
component to make the getter work without breaking existing code
that is using getValueByName with the old name.
Change-Id: I650b0766a0adf3f3c915e2337e2a7ebbddd9fc72
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/199955
Tested-by: Jenkins
Reviewed-by: Neil Roberts <bpeeluk@yahoo.co.uk>
When the print dialog is created it installs an idle handler to update
the number of pages. The printDialog test was checking the label for the
number of pages immediately after the creating the dialog so there was
likely a race condition and the test sometimes got the stub “/ %n” value
instead of the actual number of pages. This is probably the cause of
occasional build failures such as this one:
https://ci.libreoffice.org/job/gerrit_linux_clang_dbgutil/198048/
If I use the gen backend locally then the test fails consistently. This
patch fixes it by just waiting for idle with waitUntilAllIdlesDispatched
after creating the dialog.
Change-Id: I26919c90a85258169412e80fa472d199e329417b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/198349
Reviewed-by: Neil Roberts <bpeeluk@yahoo.co.uk>
Tested-by: Jenkins
Now that the restriction on passing on Any as an argument to a method
has been lifted we can simplify some of the PythonTests that were
calling uno.invoke in order to pass an Any and just call the method
directly.
Change-Id: I14af4e3f50236e46f6108f8fea0284f696b59e44
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/195464
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <stephan.bergmann@collabora.com>
Previously when creating a callable object for an attribute on an
object (ie, a method), it would mark it with a special mode to disallow
passing a pyuno.Any as an argument. There doesn’t seem to be a good
reason to do this and it makes it difficult to use an explicit type when
calling a method that can take an Any. This patch instead just removes
the mode parameter from pyobject2Any entirely.
The example in the bug report is trying to call
createInstanceWithArgumentsAndContext which takes a Sequence of Anys as
an argument. Without this patch it is difficult to pass a null interface
as one of the arguments.
Change-Id: I8756bdc2821ebf247ff67a66f071263ac00e3890
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/195452
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <stephan.bergmann@collabora.com>
Most of the uses of the EventListener wait for the executed flag to be
set in a check-sleep loop. If the code then tries to access properties
of the listener such as the supplements array then there was a race
condition where the waiting thread might wake up in between the time
that the executed flag is set and the details are filled in. This can
happen for example in open_subcomponent_through_command. I can replicate
this problem locally by adding a short sleep after setting executed to
True.
This is likely the cause of build failures such as the following:
https://ci.libreoffice.org/job/gerrit_linux_clang_dbgutil/194139/
This patch just changes the order so that the executed flag is only set
after the event details are filled in.
Change-Id: I9ba108bc2e2fa084fd5d9e3048e34203feaa74a0
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/195483
Tested-by: Jenkins
Reviewed-by: Neil Roberts <bpeeluk@yahoo.co.uk>
When using the CPPUNITTRACE make variable, the makefiles try to sneak in
some arguments to the debugger to set the library path and some other
things. It also set some arguments for malloc debugging which are
hardcoded to be in gdb format for Unix platforms and lldb format for OSX
platforms. That means that it won’t work if you use
CPPUNITTRACE="lldb --" on Linux. This patch tries to fix that by instead
making the malloc variables just be a debugger-neutral list of
variables. That is then mapped to the right command for the chosen
debugger using the $(foreach) function.
Change-Id: Iebe131365c57e987deafaa7878f2f6dfebd4b6d0
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194893
Reviewed-by: Neil Roberts <bpeeluk@yahoo.co.uk>
Tested-by: Jenkins
This is similar to the brokenCSV test except that the data is valid and
we check whether the import actually worked. The main reason to do this
is to check that the “Run the import directly on the main thread patch”
hasn’t broken the imports. Ideally this would test importing XML too,
but that doesn’t seem to work as described in tdf#169574.
Change-Id: I0ab4d566c28e4c94c0865cfc2a5e894b24927d64
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194937
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
When Import is called with mbDeterministic set to true, all of the data
providers would previously launch the import thread and then immediately
join the thread thus tying the main thread to the import thread. This is
equivalent to just doing the work directly on the main thread because in
either case the main thread is blocked until the work is complete.
However, normally when the main thread tries to acquire the solar mutex
it will additionally process any pending yield requests from other
threads. In this case this yield processing doesn’t happen because the
import thread returns false for IsMainThread even though in practice
it’s working as if it’s the main thread because the real main thread is
waiting on it.
This patch instead makes it just run the import code directly in the
current thread when mbDeterministic is true instead of launching the
other thread. This is probably slightly more efficient, but the main
reason to do this is to fix UI tests that invoke this code. In that case
UIObjectUnoObj::executeAction will call Scheduler::ProcessEventsToIdle
after sending the click event but without this patch that function’s
attempt to cause the main thread to yield will never return when using
the SVP backend. That happens because that backend doesn’t release the
solar mutex when waiting for the main thread to yield and the import
thread doesn’t yield when trying to acquire the mutex because it isn’t
the main thread. Instead deadlock occurs.
Change-Id: I0c0cf4542f899118e5b6da8bb68b4d645e3fe40e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194916
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
This reverts commit b685c7d926.
It looks removing --oneprocess didn’t help in the end. I haven’t seen
the test fail since the commit below so hopefully we can assume the
problem is fixed:
commit 9b004694bb
Author: Neil Roberts <bpeeluk@yahoo.co.uk>
Date: Sun Nov 23 00:52:19 2025 +0100
dbaccess/uitest: Wait for idle after selecting view mode
Change-Id: Iab7f20798d988ba9bb2e50f8ae5f46e135d5f3ab
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194617
Tested-by: Jenkins
Code-Style: Neil Roberts <bpeeluk@yahoo.co.uk>
Reviewed-by: Neil Roberts <bpeeluk@yahoo.co.uk>
The .uno:DBViewTables and .uno:DBViewQueries commands are executed
asynchronously by posting an event to the main thread from
OApplicationController. This creates a race condition for the test where
the following call to .uno:SelectAll silently fails if the event is not
processed before the command is dispatched. After that the attempt to
open a new subcomponent fails, the OnLoad event is never sent and the
test gets stuck in an infinite loop until the kill-wrapper kills it.
This happens for example in the following build log:
https://ci.libreoffice.org/job/gerrit_linux_clang_dbgutil/193243/console
I think I can artificially reproduce the problem locally by adding a
sleep in OApplicationController::OnSelectContainer and at least in that
case this patch appears to fix it. It just makes the tests wait until
the main thread is idle after issuing the commands to switch the view.
Change-Id: If8698114c30cf4219310a9c1b2a2bb365debea8c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194376
Reviewed-by: Neil Roberts <bpeeluk@yahoo.co.uk>
Tested-by: Jenkins
The Base UI tests all need to open a window that operates on the same
database, such as opening the table editor etc. There seems to be a few
race conditions related to opening sub windows that occasionally make
the tests fail so the idea with this patch is to make a central place in
test.py to add code to handle opening the window and closing it when
finished. That way we can add any synchronisation needed in a central
place.
For getting access to the new frame, the three current tests were all
trying to handle this in different ways. insert_relation waits for the
OnSubComponentOpened event, create_join_undo_redo was waiting until the
current focus window changes and the edit_field test was just crossing
its fingers and hoping that the timing worked. This last one is the
likely culprit of the following Jenkins build error for the edit_field
test:
https://ci.libreoffice.org/job/gerrit_linux_clang_dbgutil/193148/
The new helper method always listens for OnSubComponentOpened and then
extracts the new frame from that event.
When closing the window the shared code now always calls
waitUntilAllIdlesDispatched. .uno:CloseWin ends up being executing
asynchronously on the main thread which means that the later call to
.uno:CloseDoc can happen simultaneously. If --oneprocess is not used
then it will even end up directly calling dispose on the main frame
instead of calling CloseDoc. I haven’t been able to replicate the crash
locally, but is seems like this might be related to crashes like the one
below:
https://ci.libreoffice.org/job/gerrit_linux_clang_dbgutil/193204/
Change-Id: Iab0a46c3ca9f8c9eb9ac7f4fecb07f633bd0b312
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194350
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
Some of the events have really useful supplements. This will be used
later to extract the frame from the OnSubComponentOpened event.
Change-Id: Id62e8bf7d4658aa4df8920992a622f51e007ff41
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194349
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
The existing methods of XUITest to execute a command always use the
active frame to dispatch the command. All of the dbaccess UI tests end
up creating subcomponents (such as the table edit view) and need to send
commands to that. The frames for those are not the “active” frame but
only the “current” frame. All of the tests have ended up making a copy
of a helper function to dispatch to a particular provider. To avoid that
duplication this patch moves it to a common helper directly in XUITest.
This will also make it possible to use the method from test.py in a
later patch.
Note that the order of the arguments has been swapped around to match
that of comphelper::dispatchCommand.
Change-Id: I274fe12e4dfec0bf78359d0e2b70a3cf4db5528c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194348
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
This is a follow-up to the commit below:
commit a3546f2a04
Author: Neil Roberts <bpeeluk@yahoo.co.uk>
Date: Tue Nov 18 14:26:42 2025 +0100
uitest: Wait for idle when calling close_doc for a database doc
I realised that it’s possible to trigger the race condition with the
existing tdf99619_create_join_undo_redo test too. That one works by
loading an existing document instead of creating an empty database so it
isn’t fixed by the above commit. This patch moves the call to
waitUntilAllIdlesDispatched into close_doc instead so that it will
affect all UI tests. I can’t seem to trigger any race conditions with
non-database tests but on the other hand I don’t think those tests are
explicitly intending to test concurrent UI access so I don’t think it
does any harm to make them wait as well.
Change-Id: Icd49518fc8e7682a61548c1dd906bff2ed26d3c6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194241
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
When a signal is received such as when the unit test is killed by the
kill-wrapper, the framework catches the signal and terminates the
connection to LibreOffice. However it would then continue executing the
test. This makes it difficult to diagnose problems when the kill-wrapper
is invoked. This patch makes it instead throw an exception so that the
build log will have a backtrace. It also changes the resetSignalHandler
method to set the signal handlers to the DEFAULT rather than IGNORE,
because ignoring all subsequent signals seems like an unusual thing to
do and there is no comment explaining it so it seems likely to be a
mistake.
Change-Id: I8d8283cccd4de1ce55b0a6cd947c1647d28f462f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194321
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
Something seems to be making the tdf99619 test hang and hit the
kill-wrapper when run on Jenkins in the same process as the
insert_relation test. This needs some more investigation, but in the
meantime to avoid breaking Jenkins this patch makes the tests run in
separate processes.
Change-Id: I6f3806e77556115295e11cce89518f8e39deb894
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194293
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
If we don’t close the edit query window then the main database window
closes leaving the just the query window open. When the test is run
without --oneprocess for some reason this causes a crash. This needs
some more investigation but in the meantime this lets us run without
--oneprocess and avoid some issues with the test being flaky on Jenkins
when run alongside the insert_relation test.
Change-Id: Iaca5c0359bf8d29e5ecf317c0689939e92de733a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194292
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
The test creates a database with three tables and then creates a new
query involving all three tables. It does this so it can open the insert
relation dialog with more than two tables. It then selects various
values for the left table and makes sure that the right table is updated
so that it’s not the same.
Change-Id: Ifd23bb3343a415d828697719c17f5afbff55d856
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194041
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
The UITest calls .uno:CloseDoc at the end of create_db_in_start_center.
This seems to end up triggering some race condition which makes any
tests using it occasionally fail. I think what happens is this:
If the document is modified when CloseDoc is called then instead of
immediately closing the document it posts an event to the event queue
which causes the main thread to start closing the document
asynchronously. In the meantime the UI test thread continues and
executes Desktop::terminate. Both the main thread and the UI test thread
end up calling XFrameImpl::setComponent simultaneously. The main thread
does this with the solar mutex held because the event handling code
locks the mutex during the entire process. The UI test thread doesn’t
hold it, but parts of XFrameImpl::setComponent require the lock so it
blocks straight away. The first part of XFrameImpl::setComponent sets
the controller to the new one and then starts disposing the old one. The
disposing implementation for OApplicationController temporarily releases
the solar mutex while it does the potentially time consuming process of
disconnecting from the database (see the SolarMutexReleaser in
ODriverDelegator::shutdownConnection). This causes the UI test thread to
unblock and continue executing XFrameImpl::setComponent. Presumably it
bypasses trying to dispose the controller because that will have already
been set to null by the main thread. It will therefore jump straight to
disposing the old component window. This invokes VCLXWindow::dispose
which ends up disposing the ODataView with the solar mutex locked. Once
it is finished and the main thread has finished disconnecting from the
database it will be able to reacquire the solar mutex and continue
invoking OApplicationController::disposing. The next step calls
attachFrame which in turn calls attachFrame on the ODataView. However
the data view has been disposed in the meantime which makes it call a
method on a null pointer because m_pAccel is now null. This will end up
throwing an uncaught std::system_error.
I tried working around this one problem by checking whether the
ODataView has already been disposed in ODataView::attachFrame. However
this then just triggers a different problem where Base tries to close
the database connection in one thread while
ODriverDelegator::flushConnections is running in another and it also
ends up throwing a std::system_error.
Seeing as there seems to be a nest of bugs in this area this patch just
makes it wait for CloseDoc to finish before continuing the rest of the
unit test. If we want to fix the problems later we could make some
further unit tests that explicitly execute CloseDoc without waiting.
Change-Id: Ie531ada3d740e9e777d2b73ee162d4818fb6c477
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194181
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
The unit test ensures that any pending tasks have actually completed
even if they temporarily release the solar mutex. Without ChangeId
Ib5c6efe31dfb5e1a5039b43702e23bf0104cd403 the function would return as
soon as the solar mutex is released instead.
Change-Id: I23f8f365d3e8198d61f805f6b5d15b6ce098055b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194194
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Judging by the comment that was in waitUntilIdleDispatched, I think the
intention of this helper class is to wait until the main loop is idle
and that any event processing has actually completed. However, if this
is called outside of the main thread then that doesn’t work if the event
processing temporarily releases the solar mutex. In that case, the
thread that is waiting for the IdleTask flag would wake up and set the
flag itself even though the main thread hasn’t actually finished
executing its event handling code yet.
To fix that this patch makes it instead wait with a
std::condition_variable instead of Yield when not running on the main
thread so that we can be sure that the main thread will invoke the idle
task instead of the waiting thread.
Change-Id: Ib5c6efe31dfb5e1a5039b43702e23bf0104cd403
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194180
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Adds a method that creates a database document from the start center
button similar to create_doc_in_start_center. Creating a database is a
bit more involved because it first opens a dialog asking for details
about the database. It also needs a filename immediately so a temporary
file is created to store it.
Change-Id: I3e5489ce539be0d9d9148b612b25da828866256f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194040
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
Previously the dialog for inserting a relation behaved differently
depending on whether the query has two or more than two tables. If there
are only two, then you can select either table as the left table and the
right table will automatically flip to select the other table. If there
are more than two tables then instead it will remove the second table
from the list of options for the left table and remove the first table
for the list of options for the right table. As you change the values of
a combobox it would add and remove options in the other one so that you
can never select the same table in both boxes.
This behaviour is quite annoying if you happen to want to choose the
second table as the left part of the join. In that case you have to
first set the right table to something else so that it will appear in
the left combobox and let you select it. After that you can select the
table you actually wanted in the right box.
Instead of that behaviour, this patch makes it always have all the
tables available in each combobox. If you select the same one that is
already in the other combobox, it will now just shove the selection
along by one place including wrapping around back to zero if the last
one is already selected. I think this is easier to work with. It also
means we don’t have to have a special case to handle when there are only
two tables because the general case will have the same behaviour as
before the patch.
Change-Id: Ib02e4d169d7e2756ebfccd9d20a1ab244fa5688e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194039
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
A bunch of the UI tests were using only three spaces to indent the
“def” line for a method. In most of the tests, the subsequent lines were
indented by five spaces so that most of the code ends up on a multiple
of 4 spaces and it works out. A few of the tests only indented the
subsequent lines by one additional space so if we fix the “def” line we
need to also fix the rest of the function because otherwise it would end
up having the same indentation as the declaration.
This patch normalises the indentation to use multiples of 4 spaces. I
think the change is worthwhile because the tests are often copy-pasted
and the misindentation is probably spreading that way.
The change was made with this snippet of Emacs lisp:
(with-temp-buffer
(call-process "bash" nil (current-buffer) nil
"-c" "git ls-files | grep '/uitest/.*\\.py$'")
(goto-char (point-min))
(while (re-search-forward "^.+$" nil t)
(save-excursion
(find-file (match-string 0))
(when (local-variable-p 'require-final-newline)
(setq require-final-newline nil))
(goto-char (point-min))
(while (re-search-forward "^ [^ ]" nil t)
(goto-char (match-beginning 0))
(let ((end (if (looking-at " *def\\_>")
(copy-marker (save-excursion (end-of-defun) (point)))
nil)))
(insert " ")
(when end
(forward-line)
;; Find the next indented line
(when (re-search-forward "^\\( +\\)[^ ]" end t)
(let ((indent (length (match-string 1))))
;; If it’s not indented by 8 then adjust the whole
;; function by the right amount
(when (/= indent 8)
(beginning-of-line)
(increase-left-margin (point) end (- 8 indent)))))
(set-marker end nil)))))))
Change-Id: Iaec817dd5b982010f460be61950003b6ec8e42b1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193371
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
The UI test opens a database with two tables, creates a join between the
tables and then undos and redos the join.
Change-Id: I4ad03435ce9c6484f54aa563b46d868f21a47209
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193762
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
Previously when undo is invoked to undo adding a connection between two
tables OQueryTableView::RemoveConnection gets invoked which would end up
trying to add another undo action which would re-add the connection when
invoked. Because this happens during an undo invocation, the undo
manager is disabled and instead the new undo action gets destroyed as
soon as the first undo invocation completes. However, because the new
action is a delete it tries to become the owner of the deleted
connection. That means that when it’s destroyed it will call dispose on
the connection. However, the original undo action also tries to become
the owner of the connection even though it is now disposed so that it
can put the connection back if redo is invoked on the action. In that
case the first undo action would try to put back a connection which is
already disposed which ultimately leads to a crash.
In order to fix this, the RemoveConnection implementation now avoids
adding the undo action if the bDelete parameter is false. As far as I
can tell from inspecting the code the only way this can happen is if
RemoveConnection is invoked from OQueryTableView::DropConnection and
that only happens when applying the undo/redo actions. I think it
semantically makes sense not to make an undo action for the removal when
bDelete is false because the undo action assumes the connection is
either currently linked to a table or it itself is the owner, which
means that it will ultimately cause the connection to be deleted. If the
connection being removed shouldn’t be deleted then it implies there is
already another owner.
Change-Id: I69976c7fa8f035f9d1613382e7d0698311cf4b71
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193761
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
In the dialog to insert a new relation, if there are only two tables
the list box to select the two tables tries to react to one of the
selected tables changing by making the other list box choose the other
table. However it was doing this by making the other list box flip its
selection. This breaks if the dialog is reacting to the user reselecting
the same table in a list box because the other list box flips its value
even though the value in the first list box hasn’t actually changed.
To fix the problem this patch makes it explicitly use the opposite table
from the first list box instead of just flipping its value.
Change-Id: Ibf5d39de9406c4617c33c8f81a8605cb61811f6e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193760
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Instead of calling GetLocalizedChar for every character in the string to
be localized, we now just call a replacement function that returns the
offset to apply for digits. That means that in the likely case that the
offset is zero we can skip scanning the string altogether and just
return the original one.
Change-Id: I8b1ffa3ba38d5a8ed491668c7c1a242327bce626
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193462
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
Apparently U+107B5 isn’t in the version of Noto Sans used by Debian so
the test fails for them. Seeing as it doesn’t really matter what the
character is as long as it’s outside of the BMP, let’s change the test
to use a character that is available in Debian to make things easier for
the package maintainers. I checked whether the new character should be
available by downloading the DejaVu Sans package from
packages.debian.org.
Change-Id: Ie0bd8d1e141ee41e84727bf7dee4f523a65ef210
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193497
Tested-by: René Engelhard <rene@debian.org>
Reviewed-by: René Engelhard <rene@debian.org>
Tested-by: Jenkins
The three places that call GetLocalizedChar were all functions that were
doing the same thing, ie, localizing the digits in a string. Instead of
having three separate copies of this similar code, this patch stops
exporting GetLocalizedChar and instead exports a utility function that
operates on a string.
One of the three versions of the code avoided allocating a new string if
nothing changes. This new consolidated helper uses the same approach
so that the other two places can take advantage of this very likely
optimisation.
The main motivation to do this was to avoid truncating the sal_UCS4
returned by GetLocalizedChar into a sal_Unicode. The consolidated
implementation adds the converted characters with appendUtf32 instead so
that if GetLocalizedChar ever returns characters outside of the BMP then
it would work. In practice this doesn’t currently happen so it’s not an
actual problem. However the commit below makes the compiler warn when
sal_UCS4 is truncated, so this patch fixes a truncation that was marked
as a TODO in that commit.
commit 82f055e5cb
Author: Noel Grandin <noel.grandin@collabora.co.uk>
Date: Tue Nov 4 10:12:18 2025 +0200
new loplugin:narrow
Change-Id: I628a29520a0896cd0f4c7c2ff131ac763e07925b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193460
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
The test sets the bullet point character to U+107B5 MODIFIER LETTER
BILABIAL CLICK using the UI and then checks whether that point is used
in the document. The chosen character is outside of the basic
multilingual plane so it can’t be stored in a 16-bit integer variable.
Change-Id: Ibb638ebe6f13812ecb067c6fa31aba1d1e50c54f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193354
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
The SvxCharacterMap dialog is used to select a character and returns a
32-bit Unicode codepoint via GetChar. When invoked from
SvxBulletPickTabPage this was then being stored in a sal_Unicode
variable so it would get truncated to 16-bit. This patch just fixes it
to store the result in a sal_UCS4.
Change-Id: I49984817388dc8d77284f18c085804bed049d740
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193353
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
BrowseBox::AutoSizeLastColumn calls SetColumnWidth with the width set to
LONG_MAX. SetColumnWidth then tries to limit the actual width to the
remaining space for the last column. However, this was only done when
the window is visible, presumably because the remaining space
calculations might not work otherwise. This width is later used to
calculate a rectangle starting from the column’s X position. This of
course ends up overflowing the Long variable so it could be counted as
undefined behaviour. Instead, let’s just bail out early from the
function when the window isn’t visible and leave the width as it was.
The actual width will be recalculated when the window becomes visible
anyway because Window::Show calls Resize.
This was picked up by Stephan Bergmann with UBSAN and it was discussed
in the comments for this patch:
https://gerrit.libreoffice.org/c/core/+/193174
Change-Id: Ic2922a3d98c8ebc2e7f2d04c8c59105fdbd4e666
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193268
Reviewed-by: Stephan Bergmann <stephan.bergmann@collabora.com>
Tested-by: Jenkins
This makes it easier to find the two controls in UITests. Otherwise they
are buried in a hierarchy of windows that also don’t have a name so they
are difficult to get access to. I mainly wanted to do this to switch
between the selected rows via key presses. There doesn’t seem to be any
commands or any other easy way to select a row in a test.
Change-Id: Ic20ee4f2b1e416e4274a2f159a27448f4b3580ef
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193173
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Tested-by: Jenkins
I think this will be useful when you want to manipulate a window that
doesn’t have a unique ID and is difficult to get access to but you can
make it get the keyboard focus via UNO commands or keyboard shortcuts.
Change-Id: Iafc41dda7c3fdc1b15a62875fe3aa7efc88dcbe5
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193172
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
The dispose methods were only called in the destructors of the these two
classes so they didn’t work like a normal dispose method to break
reference cycles. Other than that the only useful work they did was to
hide all of the controls in OFieldDescControl. This turns out to be kind
of dangerous because if one of the controls had the keyboard focus then
it would end up sending the focus lost event which would in turn call an
undefined pure virtual function and crash.
This patch is needed to fix the crash when OFieldDescControl is used in
the new table wizard and the copy table wizard. In that case we don’t
need to try to flush the modifications because the new table is just
discarded as if the dialog was cancelled when the window is closed.
Change-Id: I56b585d2dcd5bbf2ee2f0d770a33a02685e60d0c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192977
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Previously when the table edit window was about to be closed it relied
on being able to trigger the OFieldDescControl to save its pending
changes by making it lose the keyboard focus. This doesn’t work on the
GTK3 backend and also seems like a bit of an unreliable approach anyway.
This patch instead adds a method to explicitly make the control flush
any pending changes.
Change-Id: I8e7b8e55ce4f56323fe1c0779bf3237ce847e5e8
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192976
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
Instead of having two long if-chains to handle the focus for the
controls, there is now an inline helper method that calls a function on
each of the controls. This is then used by the two handler methods to
check all of the controls. This should make it a bit easier to handle
the list of controls that need similar code.
Change-Id: I9787b92156dee39479a260d029cbd382fbef9394
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192975
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
All of the subclasses of OWidgetBase have a help text and a position so
we might as well move them into the base class.
Change-Id: Icfca13fd1d8e92cce99d835adb9f9478ed8c5502
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192974
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
OnControlFocusLost has a long if-chain of actions to perform for each
control. The last one looks like it was trying to update the format
sample whenever the default value changes. However, there’s already a
case further up the chain that handles the case where the default value
text box loses focus and the value is different from the saved value.
That means the format sample would only end up being updated if the
default value control loses focus but nothing was changed in it, which
seems pretty clearly like a mistake. It makes it so the only way to make
the format sample update after changing the default value is to first
move the focus to another control, move it back to the default value and
then move it away again.
This patch just moves the format sample update into the same branch of
the if-chain that already exists to handle the default value control.
Change-Id: Ia4885589a7f50e01479972cceb4e7c6875efbde0
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192753
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
Since e6004bddfc8d7d41f4e17691eaeefd770c608a13 the default value for a
table field is converted to a locale-independant format where the
decimal separator is always “.” using OUString::number. However when
loading the value back again to display in the text box the value was
being parsed with the decimal separator of the table field. This will
fail if the decimal separator in the format for the field is not “.”. To
fix that this patch changes it to parse the string with
OUString::toDouble instead.
I think that tdf#75509 and tdf#138409 are the same bug. I think the
commit mentioned above fixes part of the problem so that if you create a
field with a default value that has decimal places then they will be
used when creating a new row. However, if you then try to edit the field
again then without this patch the default value will be reset to a blank
string because the double parsing code doesn’t understand the “.” in the
canonicalised string.
Without this patch it’s also quite difficult to create such a field at
all. Whenever you try to save the table after editing, the text box for
the default value reloads the new value and triggers the parsing problem
so it gets reset to a blank string. The only way I can get it to work is
by selecting a different field before saving the table or closing the
edit window. Somehow this problem must have slipped through the testing
for the second bug so I don’t know if the testers did the steps
described above or if the problem is backend dependant or something.
Change-Id: Ic1d47893245e93322248542bf2decfdc34987771
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192740
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
If a toolbar is floating then when a button is clicked on it the toolbar
window will end up taking focus. This is kind of confusing if the button
does an action such as inserting a table row. In that case you want the
cursor to be visible in the document and to be able to immediately start
typing in the new row.
Note that despite the description in the bug report I think this is a
general problem on all platforms and with all toolbar buttons. For
example the cursor “disappears” in the same surprising way if you press
the button to insert a footnote.
To fix the problem this patch just makes it pass the focus to the
toolbar’s frame before dispatching the command. This is similar to what
is already done in the code for the font toolbar to handle when a new
font is selected.
Change-Id: Ifd941fb4ae295c57d465f4c8ff798913ce4a14c8
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192649
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Instead of having a bunch of nested if statements in order to initialise
xDispatch while the solar mutex is locked along with a check for whether
the initialise happened after the lock is released, there are now just
early returns on failures. This should make it a bit easier to add an
extra step for the success case while holding the mutex in a subsequent
patch.
Change-Id: I731a800598a8e31b2b18ee29b6354c5440a430dd
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192648
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
This is inspired by the MacOS implementation of GetKeyName where all the
key names are hardcoded in the function.
The main reason to do this is that if GetKeyName returns an empty string
then getPreferredKeyEventsForCommandList on
XCUBasedAcceleratorConfiguration will refuse to return any keys. This
makes testing the accelerator configurations a bit difficult when
running headless.
Change-Id: I3677192fe47932c9ad7f0c571315c8fdc0c5d069
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/191219
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
The Global and Module AcceleratorConfigurations are supposed to use the
new-style service specification where their argument is specified during
construction instead of using XInitialization. This was causing the test
to seg fault if it’s enabled. I think the service might have been
upgraded to the new style in 8f3cec9d7a.
Change-Id: I1fe7fa67561f6e0a0e594e5112796d14f4ce4c77
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/191218
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
The XCUBasedAcceleratorConfiguraton reloads the configuration for a key
when a change notification on that key is received. However if the
change event was triggered by deleting a key then the change path
doesn’t contain a command and the code was ignoring it. The
reloadChanged function seems to already almost be designed to handle the
case when there is no command in the configuration. In that case it
deletes the key which seems like the right thing to do. However, when a
new key is assigned an event first gets fired for the key with no
locale, so the code needs to additionally handle that case now.
I don’t think this has any practical implications unless it’s possible
to have two instances of an XCUBasedAcceleratorConfiguration for the
same module open at the same time. However, this patch alone can work as
an alternative way to fix the problem described in
96491f3f84 because it would cause the
deleted key from the module config to also be deleted in the global
config.
It looks like the check for sPath was added in
7492878a15.
Change-Id: Ia90a1932e3169ca28157482c3035aba7efae012a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/190972
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
The XCUBasedAcceleratorConfiguration-s get registered as config change
listeners in ModuleAcceleratorConfiguration::fillCache and
GlobalAcceleratorConfiguration::fillCache. The reloadChanged method then
finds the appropriate config based on the path of the change event.
However, the code didn’t do anything to check whether the config is
actually the one being managed by this accelerator configuration. That
meant for example that if a key was changed in the module configuration
then the global configuration would get notified and it would end up
replacing its configuration for that key with the module one. It would
also end up in the accelerator configuration for modules for any other
documents that happen to be open.
One way to manifest this bug is as follows:
• Create a new text document
• Tools → Customize
• Select the keyboard tab
• Select F7 in the list of keys
• Type “word count” in the search box
• Press “assign”
• Press OK
• Tools → Customize again
• Select F7 in the list of keys
• Press “delete”
• Press OK
Now look at the tools menu. Notice that F7 is still assigned to the word
count option. This is because the key has been copied into the global
configuration. The delete only deleted it from the module configuration.
Note that the messed up configuration is only stored in the read cache
so it doesn’t end up getting saved. If you restart LibreOffice then the
expected behaviour is restored.
This problem might have been added in e9e75d722d. It looks like before
that it might have been adding separate listeners for global and module
configs. Ie, connectToResourse has either PresetHandler::E_MODULES or
PresetHandle::E_GLOBAL.
Change-Id: Iac8f50f3668eaadc9a62f562d5923b93cc3d0721
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/190971
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
When a menu item has no shortcut its accelerator key code is set to
zero. When the GTK backend encounters this it was just ignoring the key
code and doing nothing to update the menu item. This meant that if an
accelerator is changed from one menu item to another then removing the
shortcut from the old menu item wouldn’t work. This patch fixes it to
handle that case.
The line calling g_strcmp0 looked suspicious. It was only called if
aCurrentAccel was nullptr, in which case it’s not worth calling because
a check whether aAccelerator is not nullptr would have the same effect.
I wonder if it was meant to be || instead of &&. In either case
g_strcmp0 is meant to handle null pointers so it seems like it should do
the right thing without having to check if aCurrentAccel is null.
This fixes the issues with the GTK backend mentioned in comment #8 of
tdf#144560.
Change-Id: Ica63a8666d2a4ca79a0c566dd43e7cb53ec9df67
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/190930
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
When a menu item has no shortcut its accelerator key code is set to
zero. When the MacOS backend encounters this it was just ignoring the
key code and doing nothing to update the menu item. This meant that if
an accelerator is changed from one menu item to another then removing
the shortcut from the old menu item wouldn’t work. This patch fixes it
to handle that case.
Change-Id: I38fa09958b78b9cbc4ea8fb9b222d4671c91fb92
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/190929
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
The accelerator cache maintains a 1:1 mapping for key→command and a
1:n mapping for command→keys. When a new command is set for a key it was
previously only updating the key→command map and then adding the key to
the command→keys map. It never removed the key from the command→keys map
for the old command. When the menus are eventually updated, it looks up
whether each item has a shortcut based on the item’s command. Because
the old command was never removed, this ended up making it display the
same key binding on multiple menu items.
Change-Id: I6d75834f4c49b1f5c105ec40cb27b8e73006152d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/190928
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
On MacOS, the keys to move to the beginning and end of the document are
expected to be ⌘↑ and ⌘↓. Similarly, to move between paragraphs, Alt+↑
and Alt+↓ are supposed to be used. This does work correctly in
LibreOffice via the doCommandBySelector method in salframeview.mm. The
MacOS SDK itself handles translating the key events into selectors such
as “moveToBeginningOfDocument”. These are then passed to
doCommandBySelector and LibreOffice has an implementation of each
interesting selector.
However, when the cursor is in a table of contents in a document, the
frame doesn’t have the InputContextFlags::Text option and the selector
translation code is bypassed. Instead the last known key event is
sent. For ⌘↑ and friends, by default this gets interpreted as an
accelerator to move between paragraphs like Ctrl+↑ on other
platforms. That means you can’t move to the beginning and end of the
document in the expected way on MacOS if the cursor is in a table of
contents.
This patch tries to fix that by just reassigning the default
accelerators to be what they are expected to be on MacOS. That way if
anything stops the translation to a selector from working then the usual
accelerator code will work correctly.
Sadly it still isn’t possible to reassign these accelerators to
something else because the selector code will still end up overriding
them when the cursor isn’t in a table of contents. doCommandBySelector
is actually deprecated since MacOS 10.6 so in the long run this will
probably have to be done a different way. In the short term we can’t
just remove the handlers for moveToBeginningOfDocument etc because it
breaks movement in text input fields in dialog boxes. Either way, this
patch improves the situation for MacOS users because at least if you
look at the default accelerator list in the customization dialog then
the keys will now actually do what it says they do.
These changes were made with the following bit of Emacs Lisp code:
(defun fix-accelerator (start end old-url new-url &optional add-key add-url)
(let ((end (copy-marker end)))
(goto-char start)
(when (re-search-forward (concat "^ <value\\b[^^]*\\(>\\)"
(regexp-quote old-url)
"</value>")
end
t)
(beginning-of-line)
(let ((value-node (buffer-substring (point) (match-beginning 1)))
(node-start (buffer-substring start (match-beginning 1))))
(goto-char (match-beginning 1))
(insert " install:module=\"unxwnt\"")
(forward-line)
(insert value-node
" install:module=\"macosx\">"
new-url
"</value>\n")
(when add-key
(goto-char end)
(forward-line)
(insert " <node oor:name=\""
add-key
"\" oor:op=\"replace\">\n"
node-start
" install:module=\"macosx\">"
add-url
"</value>\n"
" </prop>\n"
" </node>\n"))))
(set-marker end nil)))
(defun fix-accelerators-region (start end)
(let ((end (copy-marker end))
(replacements '(("DOWN_MOD1"
".uno:GoToNextPara" ".uno:GoToEndOfDoc"
"DOWN_MOD2" ".uno:GoToNextPara")
("DOWN_SHIFT_MOD1"
".uno:EndOfParaSel" ".uno:EndOfDocumentSel")
("DOWN_SHIFT_MOD2"
".uno:MoveDown" ".uno:EndOfParaSel")
("UP_MOD1"
".uno:GoToStartOfPara" ".uno:GoToStartOfDoc"
"UP_MOD2" ".uno:GoToStartOfPara")
("UP_SHIFT_MOD1"
".uno:StartOfParaSel" ".uno:StartOfDocumentSel")
("UP_SHIFT_MOD2"
".uno:MoveUp" ".uno:StartOfParaSel")))
(case-fold-search nil))
(while replacements
(let ((key (caar replacements))
(replacement-data (cdar replacements)))
(goto-char start)
(when (re-search-forward (concat "^ <node oor:name=\""
(regexp-quote key)
"\"")
end
t)
(forward-line)
(let ((start (point)))
(re-search-forward "^ </node>")
(beginning-of-line)
(apply 'fix-accelerator start (point) replacement-data)))
(setq replacements (cdr replacements))))
(set-marker end nil)))
(defun fix-accelerators ()
(interactive)
(let ((groups '("text.TextDocument"
"text.GlobalDocument"
"text.WebDocument"
"xforms.XMLFormDocument"
"sdb.FormDesign"
"sdb.TextReportDesign"))
(case-fold-search nil))
(let ((n groups))
(while n
(goto-char (point-min))
(re-search-forward (concat "^ <node oor:name=\"com.sun.star."
(regexp-quote (car n))
"\""))
(forward-line)
(let ((start (point)))
(re-search-forward "^ <")
(beginning-of-line)
(fix-accelerators-region start (point)))
(setq n (cdr n))))))
Change-Id: I81d7ebeaa324f1e96f70b58916f71d453a3e8f38
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/190600
Reviewed-by: Patrick Luby <guibomacdev@gmail.com>
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
The F2 key by default maps to “Cell Edit” which sets the input mode to
SC_INPUT_TABLE to start editing the cell. If you press F2 again, the key
event handling code in the input handler had a hardcoded handler for F2
which switched to SC_INPUT_TYPE mode when the mode is already
TABLE. This meant that you could press F2 to toggle between the two
modes. This is useful if you are editing an equation because sometimes
you want the cursor keys to move the text input cursor and sometimes you
want it to move the cell cursor to help type a cell reference.
This feature relies on the fact that the input handler code gets to see
the key event before the accelerator handler code. With the GTK backend
this works fine but on MacOS the accelerator code ends up eating the key
event before the input handler gets to see it. This meant that pressing
F2 always set TABLE mode instead of toggling. There’s also the slight
weirdness on all platforms that if you reassign “Cell Edit” to a
different shortcut key then it no longer works like a toggle and instead
you always have to press F2 to get to TYPE mode once you’re in TABLE
mode. It also means that if you tried to reassign F2 to a different
command then it wouldn’t work if you are editing a cell.
This patch changes the behaviour so that the toggling is done directly
in the handler for the “Cell Edit” command. The hardcoded handler for F2
in the input handler is removed. That means that the toggling will work
regardless of what key the command is assigned to and the input handler
doesn’t need to see the key before the accelerator key handler.
Change-Id: I79aa6fa515f7a720335f3535146c95eae874ea0a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/190557
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
The default menu on MacOS is displayed when there are no windows or on a
dialog box with no parent. Previously there were hardcoded shortcut keys
to create new documents. As reported in tdf#121253, one of these is ⌘A
which ends up being inconvenient because that is commonly used to select
all the text in a text box. This had the effect that if you tried to use
that shortcut in some dialogs then the dialog would seem to disappear
and be replaced by the new database wizard.
The hardcoded shortcuts were added in 6efdd14448 which is a patch from
January 2025. Before that the shortcuts were generated based on the menu
item title. ⌘A ends up being used for the database because ⌘D is already
used for a new Draw document and it just picked the next letter along.
These shortcuts were added in 2008 with ba57ba32e8.
This patch removes the shortcuts for the default menu in both the copied
version of the start center menu and the original default menu generated
in code. It seems strange to have shortcuts for these only in the
specific case of there being no window or when a few select dialog boxes
are focused. If they were generally useful to people then surely it
would make sense to add them to the start center menu as well. For
people using non-English language packs the patch from January would
have effectively changed the shortcut keys. If nobody has complained
then it probably means they aren’t used.
Change-Id: I6065c3daa4e288caa98d73a3f286935dc5bc07ee
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/190522
Tested-by: Jenkins
Reviewed-by: Patrick Luby <guibomacdev@gmail.com>
Previously whenever SidebarTxtControl sees a keypress it was calling
MakeVisible to scroll the comment into view. This happens even if the
keypress doesn’t do anything such as pressing shift. This patch makes it
instead only scroll the comment into view if PostKeyEvent returns true.
An example of a usecase where this was particularily incovenient is when
pressing Alt+Tab to look at another window. The initial alt press will
make the view scroll back to the last comment you were editing. This is
annoying if you are proofreading a long document while making
occassional comments because you end up losing your place in the
document.
Note that, at least on GTK, typing actual text into the comment doesn’t
make it scroll into view regardless of whether this patch is applied.
That seems to be because the text input is handled with IM commands.
That should probably be fixed too but I think it’s independent of this
change.
Change-Id: I324debee9ee2cc9d220a2b056362bea27da43e1c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/189884
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
Neil Roberts