Bug Hunting Session
Bug 108643 - Strange placeholders appear for charts after loading
Summary: Strange placeholders appear for charts after loading
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
5.3.0.3 release
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:6.0.0
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2017-06-20 02:24 UTC by Aron Budea
Modified: 2017-06-26 15:17 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
Document with charts (280.64 KB, application/vnd.oasis.opendocument.text)
2017-06-20 02:24 UTC, Aron Budea
Details
Screenshot with placeholders (155.71 KB, image/png)
2017-06-20 02:25 UTC, Aron Budea
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Aron Budea 2017-06-20 02:24:26 UTC
Created attachment 134147 [details]
Document with charts

Open the attached document that contain many charts.
Scroll through it quickly right after it finished loading.

=> Strange placeholder images appear in place of charts (see screenshot, the quicker the scrolling the worse it looks).
After scrolling back and forth the placeholders are replaced with the charts themselves (happens almost right away in this document, but can take a while in more complex ones).

Reproduced with LO 5.4beta2, 5.3.0.3 / Windows 7. Not reproduced with 5.2.0.4. => regression

Bibisecting using repo bibisect-win32-5.3 pointed to the commit referenced below. Adding Cc: to Mohammed Abdul Azeem and Michael Meeks. Please take a look sometimes.

https://cgit.freedesktop.org/libreoffice/core/commit/?id=5678bc99fb685fe09191e8419a1121e565f97f80
author		Mohammed Abdul Azeem <azeemmysore@gmail.com>	2016-09-25 05:36:32 (GMT)
committer	Noel Grandin <noel.grandin@collabora.co.uk>	2016-09-25 13:22:51 (GMT)

"tdf#101935 and tdf#102201: This fixes both the bugs."
Comment 1 Aron Budea 2017-06-20 02:25:34 UTC
Created attachment 134148 [details]
Screenshot with placeholders
Comment 2 Aron Budea 2017-06-20 02:35:24 UTC
Note an additional detail:
Since the mentioned commit, when the charts are finally displayed after scrolling around, the document becomes modified and the save confirmation dialog appears upon closing.
Comment 3 Xisco Faulí 2017-06-20 07:21:36 UTC
Confirmed in

Version: 6.0.0.0.alpha0+
Build ID: 08f6f9dded1b142b858c455da03319abac691655
CPU Threads: 4; OS Version: Linux 4.8; UI Render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group
Comment 4 Mohammed Abdul Azeem 2017-06-24 23:39:15 UTC
Actually, this bug is caused by https://cgit.freedesktop.org/libreoffice/core/commit/?id=74844277cc2194c9e43f5bd7a6f78a9603da32f3
author	Caolán McNamara <caolanm@redhat.com>	2016-09-13 14:26:41 (GMT)

Specifically, by this modification:
-        changeState( embed::EmbedStates::RUNNING );
+        awt::Size aOrigSize = getVisualAreaSize(nAspect);
+        changeState(embed::EmbedStates::RUNNING);
+        if (aOrigSize != getVisualAreaSize(nAspect))
+            setVisualAreaSize(nAspect, aOrigSize);

We had some reference issues created by my earlier commit https://cgit.freedesktop.org/libreoffice/core/commit/?id=4ccd991f6a6ca680ac2b7513ab3853e1ba9c71a3
which was somehow masking this bug and it resurfaced when we fixed that in the commit mentioned in an above comment.

I'm adding Cc to Caolán McNamara, please do take a look.
Comment 5 Mohammed Abdul Azeem 2017-06-24 23:40:06 UTC Comment hidden (obsolete)
Comment 6 Michael Meeks 2017-06-26 08:46:23 UTC
Interesting research Mohammed - any thoughts Caolan ?
Comment 7 Caolán McNamara 2017-06-26 13:34:22 UTC
The reason I made those changes was CVE-2017-3157 and it changes the time that charts are updated from during load to after load so that the security setting/question for updating links can be used.

The specific change for restoring  the size of the preview is because writer (at least, maybe others) will change the size of the object when activated, which is fine if the size is changed to the desired size afterwards as happened originally during initial load, but no use if the update happens after the load.

Hard to know what then goes wrong in chart land to get the weird placeholder problem, but if its just chart then chart has a long history of getting special cased in the embedded object code (e.g. #i103460# etc) so I can try special casing this
Comment 8 Commit Notification 2017-06-26 15:16:14 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

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

Resolves: tdf#108643 don't restore orig size on first chart activate

It will be available in 6.0.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.