Bug 128877 - FILEOPEN DOCX: Image Location lost on Import
Summary: FILEOPEN DOCX: Image Location lost on Import
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
6.2.5.2 release
Hardware: All All
: medium normal
Assignee: Miklos Vajna
URL:
Whiteboard: target:7.1.0 target:7.0.4
Keywords: bibisected, bisected, regression
Depends on:
Blocks: Writer-Images OOXML-support-crop-shape-regressions
  Show dependency treegraph
 
Reported: 2019-11-18 16:09 UTC by Luke
Modified: 2020-11-06 15:42 UTC (History)
8 users (show)

See Also:
Crash report or crash signature:


Attachments
Screenshot 6.1 vs 6.5 (232.21 KB, image/png)
2019-11-18 16:09 UTC, Luke
Details
DOCX compared MSO LO (169.13 KB, image/png)
2020-03-31 09:56 UTC, Timur
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Luke 2019-11-18 16:09:01 UTC
Created attachment 155919 [details]
Screenshot 6.1 vs 6.5

In multiple .docx files, I've noticed that LO 6.2+ is not correctly importing image locations. 

Steps to reproduce:
1. Open attachment 81684 [details] in LO 6.1
2. Open attachment 81684 [details] in LO 6.2
3. Compare

Expected Results:
1. Top left image should be at 
     Horizontal - 0.05" from Left
     Vertical - 0.64" from Top


Actual  Results:
1. Top left image should be at 
     Horizontal - 0" from Left
     Vertical - 0" from Top


Version: 6.2.8.2 (x64) Build ID: f82ddfca21ebc1e222a662a32b25c0c9d20169ee
bad

Version: 6.1.0.1 (x64) Build ID: 378e26bd4f22a135cef5fa17afd5d4171d8da21a
good
Comment 1 Xisco Faulí 2019-11-18 22:06:07 UTC
Regression introduced by:

author	Tamas Bunth <tamas.bunth@collabora.co.uk>	2019-05-13 01:02:07 +0200
committer	Tamás Bunth <btomi96@gmail.com>	2019-05-13 18:14:26 +0200
commit f4ba484183a1e7b9824f10580d633466c266828f (patch)
tree 5795d2c442e3dbb3496fcfd5ac6f5bd8ddc39e15
parent 8401a26363bf7cb3c30bf783b3f8978f4b69e4c4 (diff)
ooxml import: supprt cropping to shape

Bisected with: bibisect-linux64-6.3

Adding Cc: to Tamas Bunth
Comment 2 Timur 2020-03-31 09:56:00 UTC
Created attachment 159180 [details]
DOCX compared MSO LO

Repro in LO 7.0+.
Commit f4ba484183a1e7b9824f10580d633466c266828f had multiple regression reports I put in See Also.
Comment 3 Justin L 2020-07-17 15:10:02 UTC
also noticed with attachment 79569 [details] from bug 64789
Comment 4 Justin L 2020-07-20 16:53:01 UTC
IMHO, these kinds of patches should be reverted as soon as it is clear that the author is not going to responds (in two or three weeks). Loosing image positioning is a pretty bad regression.

Perhaps the NISZ guys are willing to investigate this. It is probably a somewhat easy fix for a shapes person to fix, and they have been doing a lot of work in this area.
Comment 5 Justin L 2020-07-20 17:02:06 UTC
For the record, the backport landed in 6.2.5.  6.2.8 was tagged on 2019-10-10, so this bug report would have been too late to ever fix 6.2.
Comment 6 BogdanB 2020-10-19 09:14:38 UTC
Also in
Version: 7.1.0.0.alpha0+
Build ID: f21f3d094cfb495c89dfb0a23ecb061ef1a2178e
CPU threads: 4; OS: Linux 5.4; UI render: default; VCL: gtk3
Locale: ro-RO (ro_RO.UTF-8); UI: en-US
Calc: threaded
Comment 7 Timur 2020-10-19 09:51:25 UTC Comment hidden (obsolete)
Comment 8 Xisco Faulí 2020-10-19 14:18:34 UTC
(In reply to Timur from comment #7)
> (In reply to Justin L from comment #4)
> > IMHO, these kinds of patches should be reverted as soon as it is clear that
> > the author is not going to responds (in two or three weeks). Loosing image
> > positioning is a pretty bad regression.
> 
> Xisco, please see. I agree with Justin (I don't know what's the value of the
> commit, also no explanation or reference).
> Affects: bug 132539, bug 64789.
> Fixed regression: bug 128686.
> Open regressions: bug 128877, bug 127086, bug 128501.
> I think Tamas was last seen in 03.2020.
> This is one of maybe 3 commits that deserves 6.4.7.3. At least it should be
> reverted in Still, if not in master.

Hi Timur,
Unfortunately at this point it's not possible to revert it, there is already another commit with a unittest < https://cgit.freedesktop.org/libreoffice/core/commit/?id=0efccbc58b41008b03f5b1ecc0a97cfcd1d4eafd > and fix for bug 128686 depends on it. I've also found that if it's reverted, unittest for bug 134174 also fails, which also contains a cropped image.

@Miklos, since Tamas Bunth is no longer part of Collabora, is it possible someone from Collabora could take a look at this issue ?
Comment 9 Miklos Vajna 2020-10-26 11:32:59 UTC
I'll try to look at this in the next few weeks.
Comment 10 Commit Notification 2020-11-03 20:31:20 UTC
Miklos Vajna committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/80aba00e4945b106a276acf4ea28309b16e7c088

tdf#128877 DOCX import: fix lost position of image cropped to shape

It will be available in 7.1.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.
Comment 11 Commit Notification 2020-11-04 09:56:05 UTC
Miklos Vajna committed a patch related to this issue.
It has been pushed to "libreoffice-7-0":

https://git.libreoffice.org/core/commit/2b17a5d63a93a5547beec2fd0d751b5995ba55fb

tdf#128877 DOCX import: fix lost position of image cropped to shape

It will be available in 7.0.4.

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.
Comment 12 Timur 2020-11-04 10:57:58 UTC Comment hidden (me-too)
Comment 13 Timur 2020-11-04 11:04:43 UTC
Miklos, image from DOCX attachment 81684 [details] is good positioned now, but with your commit it also affected "Summary" heading bellow, please see it.

Attachment 79569 [details] from bug 64789 looks good now.
Comment 14 Timur 2020-11-04 11:13:23 UTC
(In reply to Timur from comment #13)
> Miklos, image from DOCX attachment 81684 [details] is good positioned now,
> but with your commit it also affected "Summary" heading bellow, please see
> it.
More precisely:
before regression commit image was OK and "Summary" NOK
after regression commit image was NOK and "Summary" OK
after fix commit image is OK and "Summary" NOK
Comment 15 Miklos Vajna 2020-11-04 11:23:32 UTC
Please file a follow-up, non-regression bug for this new problem. This "worked" in the middle stage only by causing this bug, so in fact this never really worked. Thanks.
Comment 16 Xisco Faulí 2020-11-06 15:42:46 UTC
Verified in

Version: 7.1.0.0.alpha1+
Build ID: d7bcf59a51a058ba57e685cfc3e000fec623f716
CPU threads: 4; OS: Linux 5.7; UI render: default; VCL: gtk3
Locale: en-US (en_US.UTF-8); UI: en-US
Calc: threaded

@Miklos, thanks for fixing this issue!