Bug 128447 - XLSX: comment is displayed by default even without the <x:Visible/> tag
Summary: XLSX: comment is displayed by default even without the <x:Visible/> tag
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
6.2.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Justin L
URL:
Whiteboard: target:6.4.0 target:6.3.4
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2019-10-29 09:28 UTC by JM López
Modified: 2019-11-05 10:15 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
Test case generated with PHPSpreadsheet (7.28 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2019-10-29 09:28 UTC, JM López
Details

Note You need to log in before you can comment on or make changes to this bug.
Description JM López 2019-10-29 09:28:16 UTC
Description:
I have been trying to create a XLSX file with PHPSpreadsheet with attached comments to the cells. Everything is fine except for the fact that the comments always appear, regardless of their stated visibility.

I have been inspecting the generated VML files and they correctly omit the <x:Visible/> tag. Furthermore, Excel opens the file just fine.

I'm attaching a simple test case which works in Excel (comment starts out hidden) but not in LibreOffice (comment starts shown). 

Note that if I hide the comment in LO, save back to XLSX and reopen it, it does start out hidden, but I am not familiar enough with the XLSX format internals to pinpoint the exact modification that allows it to work.

Using LO 6.2.7.1 in Ubuntu 19.04. Can be reproduced in safe mode.

Steps to Reproduce:
1. Open the attached XLS file in LibreOffice

Actual Results:
The comment starts visible

Expected Results:
The comment starts hidden (as it should)



Reproducible: Always


User Profile Reset: Yes


OpenGL enabled: Yes

Additional Info:
Versión: 6.2.7.1
Id. de compilación: 1:6.2.7-0ubuntu0.19.04.1
Subprocs. CPU: 4; SO: Linux 5.0; Repres. IU: predet.; VCL: gtk3; 
Configuración regional: es-ES (es_ES.UTF-8); Idioma de IU: es-ES
Calc: threaded
Comment 1 JM López 2019-10-29 09:28:56 UTC
Created attachment 155388 [details]
Test case generated with PHPSpreadsheet
Comment 2 Xisco Faulí 2019-10-29 10:41:45 UTC
Regression introduced by:

https://cgit.freedesktop.org/libreoffice/core/commit/?id=2cae2ecfef47d8dd10647c10f9577392c1887d3a

author	Justin Luth <justin.luth@collabora.com>	2018-10-04 17:55:42 +0300
committer	Justin Luth <justin_luth@sil.org>	2018-10-04 21:29:23 +0200
commit	2cae2ecfef47d8dd10647c10f9577392c1887d3a (patch)
tree	f4a8910c57707a6709107d8738d50455cc0ee90e
parent	34dc3c02f6d529f5a6a3c7ae0c6d564dda23072d (diff)
tdf#120301 oox: lclIsWhiteSpace should return true for a space

Bisected with: bibisect-linux64-6.2

Adding Cc: to Justin Luth
Comment 3 Justin L 2019-10-29 18:29:49 UTC
The parser is hitting an exception when reading drawings/vmlDrawing1.vml. It didn't have an exception earlier.

oox/source/core/xmlfilterbase.cxx:414: XmlFilterBase::importFragment - XML parser failed in fragment 'xl/drawings/vmlDrawing1.vml'
Comment 4 Justin L 2019-10-30 08:33:34 UTC
The xml prolog <?xml...?> wasn't accepted.  Eliminating that from the document would fix the problem for 6.2.

I'll try to backport this proposed patch to 6.3
https://gerrit.libreoffice.org/81730
Comment 5 JM López 2019-10-30 11:33:31 UTC
Regarding #4: I have tried removing the <?xml ... ?> header from the comments1.vml file and repacking the XLSX as suggested, and the comment is still visible. So perhaps there are other things at play.

If/when the fix makes it to a nightly, I would be happy to test it.
Comment 6 Justin L 2019-10-30 11:37:19 UTC
(In reply to JM López from comment #5)
> Regarding #4: I have tried removing the <?xml ... ?> from comments1.vml
Not that one. from xl/drawings/vmlDrawing1.vml
Comment 7 JM López 2019-10-30 11:49:07 UTC
(In reply to Justin L from comment #6)
> Not that one. from xl/drawings/vmlDrawing1.vml

Sorry, I misunderstood. I can confirm that the above does work.
Comment 8 Commit Notification 2019-10-31 03:34:16 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/32efd4529aba776eca9456e96656d542267874f3

tdf#128447 sc/vml: accept <? ?> xml prolog

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.
Comment 9 Xisco Faulí 2019-10-31 09:33:39 UTC
Verified in

Version: 6.4.0.0.alpha1+
Build ID: 2d0a4182712673d8f7a5abe919cd2a1d5ece4a77
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US
Calc: threaded

@Justin Luth, thanks for fixing this issue!!
Comment 10 Commit Notification 2019-10-31 09:35:14 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-6-3":

https://git.libreoffice.org/core/commit/fd0a32ec2654b0d6cc11883991dd2f4a85ae0a44

tdf#128447 sc/vml: accept <? ?> xml prolog

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