Bug 83306 - FILEOPEN: Artifacts when importing .docx files with Autoshape images (Windows only)
Summary: FILEOPEN: Artifacts when importing .docx files with Autoshape images (Windows...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Draw (show other bugs)
Version:
(earliest affected)
4.4.0.0.alpha0+ Master
Hardware: Other Windows (All)
: highest major
Assignee: Not Assigned
URL:
Whiteboard: target:5.3.0 target:5.2.3 target:5.1.6
Keywords: bibisected, bisected, filter:docx, regression
: 82658 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-08-31 18:04 UTC by Luke
Modified: 2016-09-29 21:23 UTC (History)
10 users (show)

See Also:
Crash report or crash signature:


Attachments
Comparison of Writer 4.3 vs 4.4 (157.79 KB, image/png)
2014-08-31 18:04 UTC, Luke
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Luke 2014-08-31 18:04:36 UTC
Created attachment 105497 [details]
Comparison of Writer 4.3 vs 4.4

This bug was discovered in a LO 4.4 running in windows 7. LibreOffice 4.4 importer puts extra lines in  MS Office docx files with autoshapes. I have attached an example.

Steps to reproduce the bug:
1. Open attachment 81230 [details] AutoShapedrawing.docx document in Writer 4.3 
2. Open attachment 81230 [details] AutoShapedrawing.docx document in Writer 4.4.x
3. Compare the documents.

Note the additional stray line in Windows that does not exist in Linux. Ever since LO started supporting AutoShapes, it has not handled nested shapes properly(Bug 66058), but this regression in Windows makes it look even worse.
Comment 1 Jacques Guilleron 2014-08-31 21:50:22 UTC
Hi Luke,

I reproduce with LO 4.4.0.0.alpha0+
Build ID: 37b9ea92ba81d74764a2345a9c75c65bfd272d2b
TinderBox: Win-x86@42, Branch:master, Time: 2014-08-26_09:37:01
& Windows 7 Home Premium

regards,

Jacques
Comment 2 Xisco Faulí 2014-10-30 14:16:54 UTC
I change the software field to Windows (All) as this can't be reproduced in Linux
Comment 3 retired 2015-01-12 08:48:43 UTC
just another data point:
no repro on OSX 10.10.1 LO 4.4.2 and 4.5.0 (from today)
Comment 4 Luke 2015-09-11 01:44:49 UTC
If you click on the anomaly to try to remove it, you get the following error:
"Access violation – No RTTI Data!"
Comment 5 Julien Nabet 2015-11-08 20:51:22 UTC
Any update with last stable LO version 5.0.3?
Comment 6 Xisco Faulí 2015-11-10 10:22:45 UTC
I can't reproduce the crash with 

Version: 5.0.2.2
Build ID: 37b43f919e4de5eeaca9b9755ed688758a8251fe
Locale: es-ES (es_ES)

on Windows 7

however, the anomaly is still present.

Same result with 

Version: 5.1.0.0.alpha1+
Build ID: 41d90d6c7c41df781ecaf7745872f20abd7e52a9
TinderBox: Win-x86@62-merge-TDF, Branch:MASTER, Time: 2015-11-10_00:18:12
Locale: es-ES (es_ES)
Comment 7 Luke 2015-11-11 08:01:25 UTC
It's no longer crashing. Changed the title back.
Comment 8 Robinson Tryon (qubit) 2015-12-14 05:43:25 UTC Comment hidden (obsolete)
Comment 9 Xisco Faulí 2016-09-13 09:48:11 UTC
I guess this can be bibisected with this repository:
http://dev-downloads.libreoffice.org/bibisect/win/bibisect_win_44.tar.xz. Adding
keyword 'bibisectRequest'
Comment 10 raal 2016-09-26 14:47:47 UTC
 git bisect log
# bad: [489ffd1df414698b6cea9ab03bf6f663b8b5af7e] source-hash-3f94c9e9ddfd807b449f3bb9b232cf2041fa12d2
# good: [8aa9fc9a0c92172593d6cd97662116a965db229d] source-hash-dea4a3b9d7182700abeb4dc756a24a9e8dea8474
git bisect start 'latest' 'oldest'
# bad: [897913acd244cb6a5d2f4c2da1d625d9b978edb6] source-hash-ac57362b23859591c088e36b7218f4a606dcf3bb
git bisect bad 897913acd244cb6a5d2f4c2da1d625d9b978edb6
# bad: [dc97f44745f96315fb6c5480705bb5d595d39c6c] source-hash-01c8962f281887db59e581906b89d027a994b52a
git bisect bad dc97f44745f96315fb6c5480705bb5d595d39c6c
# good: [a5a39af2ec486b10b26d6373f58a9b59142106c4] source-hash-a595879302e26a616131aa0cab5de31bb287b37d
git bisect good a5a39af2ec486b10b26d6373f58a9b59142106c4
# bad: [54a9a5df46b192bd5b9bd44702ef3bffd5730523] source-hash-e2f1fffc81896c0773a18e468635056710927d8f
git bisect bad 54a9a5df46b192bd5b9bd44702ef3bffd5730523
# bad: [54a9a5df46b192bd5b9bd44702ef3bffd5730523] source-hash-e2f1fffc81896c0773a18e468635056710927d8f
git bisect bad 54a9a5df46b192bd5b9bd44702ef3bffd5730523
# bad: [54a9a5df46b192bd5b9bd44702ef3bffd5730523] source-hash-e2f1fffc81896c0773a18e468635056710927d8f
git bisect bad 54a9a5df46b192bd5b9bd44702ef3bffd5730523
# good: [f1cbe8b4bb7351ee7ca566e6c29229948fcac423] source-hash-9263b101c39172cbcf04189c515bca80cb15f3aa
git bisect good f1cbe8b4bb7351ee7ca566e6c29229948fcac423
# good: [649737e10cac6fffed69f0b6f02d517ca5ff2a05] source-hash-c38be106ff1c1ff5e1b279e4ea2c710b524d23f1
git bisect good 649737e10cac6fffed69f0b6f02d517ca5ff2a05
# bad: [b9e32c086d6e3c96eb05ad80b0005afeb43305c8] source-hash-81df594b4fbb147d3e4b3cb31ae27ff7f66d83b4
git bisect bad b9e32c086d6e3c96eb05ad80b0005afeb43305c8
# bad: [30fbacc51969fa4e3f572b25142b58103956ed06] source-hash-da36ded02c67bb7481cd4378ce5f7d779c1a3533
git bisect bad 30fbacc51969fa4e3f572b25142b58103956ed06
# good: [1d9fe19e246fed2d4e059300c6c19232f64659cb] source-hash-1f90cae1debed4b45bb51ced21a03aacd7973cff
git bisect good 1d9fe19e246fed2d4e059300c6c19232f64659cb
# good: [1d9fe19e246fed2d4e059300c6c19232f64659cb] source-hash-1f90cae1debed4b45bb51ced21a03aacd7973cff
git bisect good 1d9fe19e246fed2d4e059300c6c19232f64659cb
# good: [e33eab517819eed0b577bb43de03305a9d2c3aa5] source-hash-6f92b58987f754de31c9ca756e813deb7462d98e
git bisect good e33eab517819eed0b577bb43de03305a9d2c3aa5
# first bad commit: [30fbacc51969fa4e3f572b25142b58103956ed06] source-hash-da36ded02c67bb7481cd4378ce5f7d779c1a3533

One of these commits
http://cgit.freedesktop.org/libreoffice/core/log/?qt=range&q=6f92b58987f754de31c9ca756e813deb7462d98e..da36ded02c67bb7481cd4378ce5f7d779c1a3533

Age	Commit message (Expand)	Author	Files	Lines
2014-07-04	fix autotext	Caolán McNamara	1	-4/+4
2014-07-04	Related fdo#77603: update Spanish autocorrection patterns.	Adolfo Jayme Barrientos	1	-5/+140
2014-07-04	Use standard library optimised routines for OUString/OString	Noel Grandin	1	-60/+178
2014-07-04	fix spelling in class name OSpecialHanldeXMLExportPropertyMapper	Noel Grandin	2	-11/+11
2014-07-04	Fix: EE_CHAR_COLOR to EE_CHAR_BKGCOLOR	matteocam	1	-1/+1
2014-07-04	Avoid possible memory leaks in case of exceptions	Takeshi Abe	6	-47/+33
2014-07-04	Drop unused #includes	Takeshi Abe	2	-4/+0
2014-07-03	fdo#76803: writerfilter: fix image wrap polygon import again	Michael Stahl	1	-2/+4

Michael, can it be your commit?
author	Michael Stahl <mstahl@redhat.com>	2014-07-03 22:29:55 (GMT)
committer	Michael Stahl <mstahl@redhat.com>	2014-07-03 22:39:36 (GMT)
commit dcbac37efebb9877a72f7c9914b63d60f46a5656 (patch)
fdo#76803: writerfilter: fix image wrap polygon import again
Comment 11 Michael Stahl (allotropia) 2016-09-28 21:37:53 UTC
it doesn't look like commit dcbac37efebb9877a72f7c9914b63d60f46a5656 is responsible - i can't build that old master with MSVC 2013 but
a) the whole Fraction class was removed before 4.4 branch off anyway
b) if i disable the WrapPolygon handling in GraphicImport.cxx completely
   i still get the same extra lines in the image

... the minimal group shape to demonstrate the bug that shows
white image on Linux and 3 grey lines on Windows is this:

        <w:pict>
          <v:group id="_x0000_s1026" editas="canvas" style="width:342pt;height:180.65pt;mso-position-horizontal-relative:char;mso-position-vertical-relative:line" coordorigin="2785,-605" coordsize="6514,3468">

            <v:shape id="_x0000_s1027" type="#_x0000_t75" style="position:absolute;left:2785;top:-605;width:6514;height:3468" o:preferrelative="f">
              <v:fill o:detectmouseclick="t"/>
              <v:path o:extrusionok="t" o:connecttype="none"/>
              <o:lock v:ext="edit" text="t"/>
            </v:shape>

            <v:shapetype id="_x0000_t34" coordsize="21600,21600" o:spt="34" o:oned="t" adj="10800" path="m,l@0,0@0,21600,21600,21600e" filled="f">
              <v:stroke joinstyle="miter"/>
              <v:formulas>
                <v:f eqn="val #0"/>
              </v:formulas>
              <v:path arrowok="t" fillok="f" o:connecttype="none"/>
              <v:handles>
                <v:h position="#0,center"/>
              </v:handles>
              <o:lock v:ext="edit" shapetype="t"/>
            </v:shapetype>

          </v:group>
        </w:pict>


... okay if i play with strtempl.cxx i find that the responsible commit is:

commit 281989007fd7dea997ed9a65f513f80b1aff67dd
Author:     Noel Grandin <noel@peralex.com>
AuthorDate: Tue Jul 1 13:17:01 2014 +0200
Commit:     Caolán McNamara <caolanm@redhat.com>
CommitDate: Fri Jul 4 09:04:05 2014 +0000

    Use standard library optimised routines for OUString/OString
    
    ..handling where possible.


urgh :(
Comment 12 Michael Stahl (allotropia) 2016-09-28 22:33:22 UTC
the bug is triggered in compare_WithLength, in

+    if (sizeof(IMPL_RTL_STRCODE) == sizeof(wchar_t))
+    {
+        // take advantage of builtin optimisations
+        sal_Int32 nMin = std::min(nStr1Len, nStr2Len);
+        sal_Int32 nRet = wcsncmp((wchar_t*)pStr1, (wchar_t*)pStr2, nMin);
+        return nRet == 0 ? nStr1Len - nStr2Len : nRet;
+    }


the problem is that the oox code uses strings with embedded null bytes
to identify shapes (why?), and wcsncmp stops at the first null byte
but the old code would compare them too.  this causes different results.
Comment 13 Caolán McNamara 2016-09-29 09:35:19 UTC
change to wmemcmp/memcmp ?
Comment 14 Commit Notification 2016-09-29 13:04:37 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=de7ed418e7813c85f192b558ff06e976eccaa54d

tdf#83306: sal: fix compare of rtl::OUString/OString containing '\0'

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 15 Michael Stahl (allotropia) 2016-09-29 19:25:51 UTC
*** Bug 82658 has been marked as a duplicate of this bug. ***
Comment 16 Commit Notification 2016-09-29 20:38:12 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-5-2":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=d47756ab1fce2aedd7658e49fa893c2e2b63286e&h=libreoffice-5-2

tdf#83306: sal: fix compare of rtl::OUString/OString containing '\0'

It will be available in 5.2.3.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 17 Commit Notification 2016-09-29 20:51:18 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-5-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=7bc09f32ce86c6ed0df00ad72451b41204e0162f&h=libreoffice-5-1

tdf#83306: sal: fix compare of rtl::OUString/OString containing '\0'

It will be available in 5.1.6.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 18 Commit Notification 2016-09-29 21:23:41 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=a0687f7e656a137cd0c1661094afc72f466ba629

tdf#83306 add unit test for compareWithLength and '\0'

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.