Linux SAL_USE_VCLPLUGIN=svp and the clipboard

Stephan Bergmann sbergman at redhat.com
Fri Apr 3 15:09:43 UTC 2020


I ran into this when trying `SAL_USE_VCLPLUGIN=gen make uicheck` on Linux.

On the one hand we have 
<https://git.libreoffice.org/core/+/fcb97088e7be05098b829913a4f392c8d91ff4a8%5E!/> 
"uitest for bug tdf#118189" introducing test_tdf118189 in 
sc/qa/uitest/calc_tests2/tdf118189.py.  It copies a row of cells (where 
A1 contains the text "On Back Order") from one Calc document to the 
clipboard.  Then it creates a fresh Calc document and does the following 
with it:

>         #4. Paste it
>         gridwin2.executeAction("SELECT", mkPropertyValues({"CELL": "A1"}))
>         self.xUITest.executeCommand(".uno:Paste")
>         #5. Cut it
>         self.xUITest.executeCommand(".uno:Cut")
>         #6. Undo
>         self.xUITest.executeCommand(".uno:Undo")
> 
>         #-> CRASH
>         self.assertEqual(get_cell_by_position(document2, 0, 0, 0).getString(), "")

What one would assume happens is that after step 4 the new document's A1 
should contain "On Back Order", after step 5 A1 should be empty, and 
after step 6 it should contain "On Back Order" again.  So the final test 
of A1 against "" should fail (and that's what happens e.g. when your run 
that test with SAL_USE_VCLPLUGIN=gen on Linux, as suggested by 
solenv/gbuild/uitest-failed-default.sh, or when you run that test on 
Windows).  What actually happens is that with SAL_USE_VCLPLUGIN=svp 
(which is the default for UITests) each Calc document (view) apparently 
has its own clipboard (see next), so step 4 actually pastes nothing, so 
the final content of A1 is indeed the empty string.

It is unclear to me why the above test checks for the empty string 
rather than "On Back Order" ever since that first commit.  Maybe just 
because it happened to succeed that way in the "default" environment for 
running UITests (i.e., on Linux and with the default 
SAL_USE_VCLPLUGIN=svp).  That we first copy a row from another document 
to the clipboard suggests that the test was written under the assumption 
that that row would indeed be pasted into the second document?  (And 
completely ignoring the issue that a UITest using .uno:Cut/Paste, which 
can interact with a system-wide clipboard IIUC, is probably a bad idea 
to begin with.)

The reason for the clipboard-per-document is 
SalInstance::CreateClipboard in vcl/source/components/dtranscomp.cxx, 
which creates a new instance on each call.  (In contrast to e.g. 
X11SalInstance::CreateClipboard in 
vcl/unx/generic/dtrans/X11_service.cxx, used by SAL_USE_VCLPLUGIN=gen, 
which keeps returning the same instance for all calls with empty arguments.)

However, if we change SalInstance::CreateClipboard to behave like 
X11SalInstance::CreateClipboard and use a single clipboard instance for 
at least all those calls with empty arguments, we run into a different 
problem:

Because, on the other hand we have 
<https://git.libreoffice.org/core/+/1b7a8277aa3e9f73ccdf15e933a1ee3b42849a44%5E!/> 
"sc lok: 1 view has 1 clipboard to transfer data".  It introduces 
ScTiledRenderingTest::testMultiViewCopyPaste in 
sc/qa/unit/tiledrendering/tiledrendering.cxx which specifically tests 
that SID_COPY/PASTE in two different views of the same Calc document do 
not interact.  (And thus would start to fail with the above change.) 
The test is part of CppunitTest_sc_tiledrendering, which only exists on 
Linux (cf. sc/Module_sc.mk), where it again defaults to 
SAL_USE_VCLPLUGIN=svp.  And again, if you run `SAL_USE_VCLPLUGIN=gen 
make CppunitTest_sc_tiledrendering` on Linux (where overriding 
SAL_USE_VCLPLUGIN for CppunitTests appears to be explicitly supported by 
gbuild, see

> # Common environment variables passed into all gb_*Test classes:
[...]
> ifeq (,$(SAL_USE_VCLPLUGIN))
> gb_TEST_ENV_VARS += SAL_USE_VCLPLUGIN=svp
> endif

in solenv/gbuild/gbuild.mk), the test fails.

Does that test check for behavior that is vital for the tiledrendering 
use case, or does it just for some reason want to verify the status quo, 
whether or not that status quo is actually use- or harmful for the 
tiledrendering use case?  (And again ignoring the issue that a 
CppunitTest using SID_COPY/PASTE, which can interact with a system-wide 
clipboard IIUC, is probably a bad idea to begin with.)

It is not clear to me what the intended behavior of 
SalInstance::CreateClipboard is, and thus how the two tests (and the 
expected tiledrendering behavior that 
ScTiledRenderingTest::testMultiViewCopyPaste apparently tests?) should 
be fixed.



More information about the LibreOffice mailing list