Date: prev next · Thread: first prev next last
2020 Archives by date, by thread · List index


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.


Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.