Description: I didn't test other components. The origin of this bug is the failing tinderbox Win-x86_64_42. In commit 3f7e8ddea89f ("Enable many more unit tests on all archs") I enabled many more unit tests an all archs to verify if any arch specific #ifdefs are still needed and fixed some fallout, Jenkins didn't catch in commit a2c665e4dd73 ("WIN fix some unit tests more generally"). But there is one problem left: Win-x86_64_42 fails with 24952 NEXT D:/lode/dev/core/sc/qa/unit/subsequent_export-test.cxx(3086) : error : Assertion 24953 Test name: ScExportTest::testRelativePathsODS 24954 NEXT assertion failed 24955 - Expression: aURL.startsWith("..") 24956 24957 Failures !!! 24958 Run: 115 Failure total: 1 Failures: 1 Errors: 0 Thorsten thankfully added a SAL_DEBUG on that box, which shows: > $ grep debug workdir/CppunitTest/sc_subsequent_export_test.test.log > debug:4516:2084: > file:///D:/lode/dev/core/sc/qa/unit/data/ods/xlsx/databar.xlsx So obviously the test fails. But it's actually broken on all architectures. Just open the bug document of the unit test: sc/qa/unit/data/ods/fdo79305.ods. It just contains a link as <text:a xlink:href="../xlsx/databar.xlsx" xlink:type="simple">/devel/libo/libo1/sc/qa/unit/data/xlsx/databar.xlsx</text:a> For me LO fails to open the link, because it resolves the wrong target. origin: /libreoffice/sc/qa/unit/data/ods/fdo79305.ods target: /libreoffice/sc/qa/unit/data/ods/xlsx/databar.xlsx - an ods too much. The unit test just tests CPPUNIT_ASSERT(aURL.startsWith("..")), which doesn't make any sense. I would expect a correct absolute URL from LO for the relative link inside the document, so it can be forwarded to whatever system "open link" tool is used. Don't know how to test, if the contained link is absolute. Maybe someone knows where to fix this bug. I really don't want to hide it again using #ifndef _WIN32, but since this all includes those security link settings and stuff, I would appreciate if someone else takes a stab at it. Steps to Reproduce: 1. Open sc/qa/unit/data/ods/fdo79305.ods 2. Ctrl+Click the link Actual Results: Error message with wrong path. Expected Results: The linked document is opened. Reproducible: Always User Profile Reset: No Additional Info: Current master. Fails unit test on Windows. Manual test fails for the kde5 backend, but from the URL I guess it's broken on all archs.
IMHO the correct test would be to test the existence of the target file and somehow check the relative URL of the document. But since the test extracts the href via xpath it should always return "../xlsx/databar.xlsx", no?
Added Eike and Markus, as they fixed bug 79305 and added the unit test. Workaround patch: https://gerrit.libreoffice.org/#/c/75172/
Jan-Marek Glogowski committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/+/dfdd2b0126a677960946c5ff2b905e95b978b2d5%5E%21 tdf#126255 disable broken relative link unit test It will be available in 6.4.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Just removed the Whiteboard tag, as the bug was not fixed by the patch, but the failing unit test assert disabled.
(In reply to Jan-Marek Glogowski from comment #0) > <text:a xlink:href="../xlsx/databar.xlsx" > xlink:type="simple">/devel/libo/libo1/sc/qa/unit/data/xlsx/databar.xlsx</ > text:a> > > For me LO fails to open the link, because it resolves the wrong target. > > origin: /libreoffice/sc/qa/unit/data/ods/fdo79305.ods > target: /libreoffice/sc/qa/unit/data/ods/xlsx/databar.xlsx > - an ods too much. Given the xlink:href="../xlsx/databar.xlsx" the target is correct, it's not one ods too much but one ../ too little in the href. The first ../ escapes the ODF package (.ods) so to target a file ../xlsx/databar.xlsx relative to the current file it should be xlink:href="../../xlsx/databar.xlsx" instead. That's exactly what my follow-up commit https://cgit.freedesktop.org/libreoffice/core/commit/?id=bc3b62e25eb0c3921fa600e80eeb314e45ecaaef to bug 79305 addressed, but the test document wasn't adapted. I have no adhoc idea why the test apparently failed only on Windows if the target doesn't exist. I'll correct the test document and revert the temporary "fix" and then remove the #if !defined _WIN32 to see if Windows boxes still stumble upon.
Or, actually the #if !defined _WIN32 was already removed with dfdd2b0126a677960946c5ff2b905e95b978b2d5 ... well, let's see.
(In reply to Eike Rathke from comment #5) > (In reply to Jan-Marek Glogowski from comment #0) > > <text:a xlink:href="../xlsx/databar.xlsx" > > xlink:type="simple">/devel/libo/libo1/sc/qa/unit/data/xlsx/databar.xlsx</ > > text:a> > > > > For me LO fails to open the link, because it resolves the wrong target. > > > > origin: /libreoffice/sc/qa/unit/data/ods/fdo79305.ods > > target: /libreoffice/sc/qa/unit/data/ods/xlsx/databar.xlsx > > - an ods too much. > Given the xlink:href="../xlsx/databar.xlsx" the target is correct, it's > not one ods too much but one ../ too little in the href. The first ../ > escapes the ODF package (.ods) so to target a file ../xlsx/databar.xlsx > relative to the current file it should be > xlink:href="../../xlsx/databar.xlsx" instead. Interesting. So ".." isn't directory-only in this context (I don't know how to express that better). Didn't know that. And maybe my match to file system semantics is completely wrong here. > I have no adhoc idea why the test apparently failed only on Windows if the > target doesn't exist. I'll correct the test document and revert the > temporary "fix" and then remove the #if !defined _WIN32 to see if Windows > boxes still stumble upon. For whatever reason the other system paths from the link start with "..". Thanks for looking into it.
(In reply to Eike Rathke from comment #6) > Or, actually the #if !defined _WIN32 was already removed with > dfdd2b0126a677960946c5ff2b905e95b978b2d5 ... well, let's see. Yup - I didn't see any point in leaving the wrong assert, so I just commented that and the test can at least test loading the document.
Eike Rathke committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/+/7ec598639a900e21ea472a85057aadee10796778%5E%21 Resolves: tdf#126255 correct xlink:href to escape package, tdf#79305 related It will be available in 6.4.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Eike Rathke committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/+/1458d43690684c5ce6ed4409bef34a23cfbab7f2%5E%21 Revert "tdf#126255 disable broken relative link unit test" It will be available in 6.4.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
(In reply to Jan-Marek Glogowski from comment #7) > Interesting. So ".." isn't directory-only in this context (I don't know how > to express that better). Didn't know that. And maybe my match to file system > semantics is completely wrong here. See 3.7 Usage of IRIs Within Packages https://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part3.html#__RefHeading__752821_826425813
(In reply to Commit Notification from comment #10) > Eike Rathke committed a patch related to this issue. > It has been pushed to "master": > > https://git.libreoffice.org/core/+/ > 1458d43690684c5ce6ed4409bef34a23cfbab7f2%5E%21 > > Revert "tdf#126255 disable broken relative link unit test" > > It will be available in 6.4.0. > > The patch should be included in the daily builds available at > https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More > information about daily builds can be found at: > https://wiki.documentfoundation.org/Testing_Daily_Builds > > Affected users are encouraged to test the fix and report feedback. Hmm, that does not seem to have helped. Win42 now fails again, with the old error: |D:/lode/dev/core/sc/qa/unit/subsequent_export-test.cxx(3086) : error : Test name: ScExportTest::testRelativePathsODS assertion failed - Expression: aURL.startsWith("..") As Jmux listed with the original description: on that setup, the URL becomes absolute - no idea why.
(In reply to Thorsten Behrens (CIB) from comment #12) > Hmm, that does not seem to have helped. Win42 now fails again, with the old > error: > > |D:/lode/dev/core/sc/qa/unit/subsequent_export-test.cxx(3086) : error : Test > name: ScExportTest::testRelativePathsODS > assertion failed > - Expression: aURL.startsWith("..") > > As Jmux listed with the original description: on that setup, the URL becomes > absolute - no idea why. This is just one part of the brokenness / mystery. On other boxes the aURL.startsWith("..") succeeds. But this means it can't be an URL, as the aURL is just an OUString, so no magic overloaded startsWith, which could somehow ignore the URL scheme. So in reality it must be a system path. But for your Windows box it starts with "file:///", which is a real URL. And your box has probably now an aURL of "file:///D:/lode/dev/core/sc/qa/unit/data/xlsx/databar.xlsx", which is at least a correct URL pointing to the correct document. But my linux box here returns this as aURL: ../../../home/jmg/Development/libreoffice/symbols/sc/qa/unit/data/xlsx/databar.xlsx That is - from my understanding - neither an URL nor a correct relative path to the xlsx from the CWD of the /home/jmg/Development/libreoffice/symbols/sc/qa/unit/data/ods/fdo79305.ods Maybe I wasn't clear in the report, but from any kind of interpretation, aURL should either always be a valid URL or a system path. The current state is just crazy.
You are confusing what the test does, it tests the xlink:href attribute value, not the final resulting URL. It passes on other platforms and the Jenkins Windows box (if that ran the test at all?) but mysteriously fails on this one Windows box.
So debugging it and with mst__ pointing it out on IRC based on my gdb session, at least my Linux path makes sense. Reminder: this failing test is an export test! So the document is loaded from * /home/jmg/Development/libreoffice/symbols/sc/qa/unit/data/ods/fdo79305.ods but it is exported to * file:///tmp/<some tmpdir>/dummyObjectName/content.xml which now results in the correct relative link of (with erAck patch to the doc): * ../../../home/jmg/Development/libreoffice/symbols/sc/qa/unit/data/xlsx/databar.xlsx And reading the RfC 3986 "4.2. Relative Reference" it's also clear that for the relative reference the schema is omitted. Now that leaves the Windows test case. I guess the TEMP directory for Windows is somewhere at file:///C:/... but the LO build directory is file:///D:/... Maybe that can't be expressed relative.
Makes sense, in that Windows different drive letters case the href has to be absolute.
Jan-Marek Glogowski committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/+/3f1527fba7b9fe729e421322a9ea91a24aa495e0%5E%21 tdf#126255 handle _WIN32 with different drive letters It will be available in 6.4.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Jan-Marek Glogowski committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/+/2f2f4767089512c34514896bc37823f9310e9dd4%5E%21 tdf#126255 compare the URL of the exported file It will be available in 6.4.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
With the last patch the test seems to pass again. Thanks for all the input, so it could be fixed properly.