Created attachment 104115 [details] LibO 4.2.6 VS 4.3.1 Steps: 1) Open attachment 104062 [details] 2) Notice that the chart title 'Chart Title' isnt there. Tested in 4.4 and 4.3.1 on Linux. Regression that started in 4.3.x.
Bodhi Linux 2.4 - E17 LibreOffice 4.4 Built August 7, 2014 Confirmed: New Major - at least for user it appears like loss of data High - default seems fine
Regression does not appear in latest version of 43all and must be younger. Add bibisected and bibisectednewer as specified in the QA Wiki
bibisect range a2aa0e6087254165613d323a5cfb083c79e7bbd9..113866583c5a34b9242aeb9040c4ce80e65da1ba regressoin from: commit 2f74e52a6efe7a51d6575cbb9b5f30b1ad99ee7c Author: Markus Mohrhard <markus.mohrhard@collabora.co.uk> AuthorDate: Sun Jun 22 04:47:36 2014 +0200 use the correct default value, fdo#78080
Let us start by realizing that this file has been written by MSO 2007 and our current import is valid according to OOXML. The funny part is that 2007 is expecting most default values not as specified by the standard. So for me this is not a real regression but an enhancement. At some point we need to implement another import filter that deals with the differences between OOXML and the MSO 2007 dialect.
imho what OOXML standard says is only of academic interest; we should do what MSO does when importing these files. would it be possible to add something like the generator-based checks in xmloff (which is used to apply fix-ups for files written by old and buggy OOo versions) to the OOXML filters too, to have the defaults depend on the MSO version that produced the file?
(In reply to comment #5) > imho what OOXML standard says is only of academic interest; > we should do what MSO does when importing these files. Modern MSO versions expect the OOXML spec behavior while 2007 and 2003 with the OOXML extension write the wrong files. You can now decide which behavior is the one that we want to implement by default. > > would it be possible to add something like the generator-based > checks in xmloff (which is used to apply fix-ups for files > written by old and buggy OOo versions) to the OOXML filters too, > to have the defaults depend on the MSO version that produced > the file? Yes. And that is what I meant with another import filter. However it is quite a bit of work as quite a bit of default values have been changed already based on bug reports. Currently I see it as low importance as more and more 2010+ documents are produced and less 2007 ones.
It seems that the commit that caused this regression was identified. (Or at least a commit is suspected as the offending one.) Thus setting keyword "bisected".
I can confirm that saving this file with MS 2013 resolves the issue with the missing chart title (on save it warns that it's upgrading to a newer OOXML). (In reply to Markus Mohrhard from comment #6) > Modern MSO versions expect the OOXML spec behavior while 2007 and 2003 with > the OOXML extension write the wrong files. You can now decide which behavior > is the one that we want to implement by default. Are you saying that we need to pick one version and can only import that version correctly? If so, this is not an acceptable answer to corporate users. I worked in IT at a company that had different versions of MS office in different departments that sent and received files from clients with a wide range of MS Office versions. The only time I ever encountered an MSO interoperability issue was with a client that used an old version of the Mac MSO. MS Office somehow manages to set the correct import/export defaults for all the major MSO versions. I agree with Michael that we must also do this. If we chose to only read one idealized version of OOXML, it will reflect poorly on us and limit the number of users that can use us to migrate away from MSO.
Added feature enhancement request Bug 90314 - filter: Add support for multiple OOXML versions Please feel free to correct any technical errors I may have made in my understanding of our current OOXML import system.
Markus has already added in code to detect the MSO 2007 dialect. https://mmohrhard.wordpress.com/2015/03/11/supporting-more-ooxml-dialects-in-chart-import/ http://cgit.freedesktop.org/libreoffice/core/commit/?id=15174177091367332b57cd79575e2f7dd27388b2 http://cgit.freedesktop.org/libreoffice/core/commit/?id=23a1717881ebfa3638b969aa4bad38a81d26d29d (In reply to Markus Mohrhard from comment #6) > Modern MSO versions expect the OOXML spec behavior while 2007 and 2003 with > the OOXML extension write the wrong files. > > You can now decide which behavior is the one that we want to implement by > default. I believe the default should be the OOXML spec and we should implement workarounds to hand the 2007/2003 exceptions. (In reply to Luke from comment #9) > Added feature enhancement request Bug 90314 - filter: Add support for > multiple OOXML versions I believe Markus's above commits handle the 2007/2003 exception, but believe it should be extended to know if its a 2010 ooxml files as well, because there are ooxml differences that show up between these versions as mentioned in bug 77794.
(In reply to Jay Philips from comment #10) > Markus has already added in code to detect the MSO 2007 dialect. > > https://mmohrhard.wordpress.com/2015/03/11/supporting-more-ooxml-dialects-in- > chart-import/ > > http://cgit.freedesktop.org/libreoffice/core/commit/ > ?id=15174177091367332b57cd79575e2f7dd27388b2 > > http://cgit.freedesktop.org/libreoffice/core/commit/ > ?id=23a1717881ebfa3638b969aa4bad38a81d26d29d There are still a few dozen cases that need to be adapted. However this has to wait until a later time as the customer document only had this one problem. > > (In reply to Markus Mohrhard from comment #6) > > Modern MSO versions expect the OOXML spec behavior while 2007 and 2003 with > > the OOXML extension write the wrong files. > > > > You can now decide which behavior is the one that we want to implement by > > default. > > I believe the default should be the OOXML spec and we should implement > workarounds to hand the 2007/2003 exceptions. > > (In reply to Luke from comment #9) > > Added feature enhancement request Bug 90314 - filter: Add support for > > multiple OOXML versions > > I believe Markus's above commits handle the 2007/2003 exception, but believe > it should be extended to know if its a 2010 ooxml files as well, because > there are ooxml differences that show up between these versions as mentioned > in bug 77794. The differences between MSO 2010 and MSO 2013 are totaly different to the chart problems with MSO 2007. They changed default values between these versions whereas the MSO 2010 and MSO 2013 versions don't require to know who produced the file. They are in the file in either MCE records or ExtLst records which we just ignore right now.
(In reply to Markus Mohrhard from comment #11) > There are still a few dozen cases that need to be adapted. However this has > to wait until a later time as the customer document only had this one > problem. True that the fix related to a single case from your customer, but the part of the code that i was addressing in my comment relates to the push having MSO 2007 detection in it, which is the primary discussion between you and Michael from comment 4 to comment 6. :D
by changing oox/source/drawingml/chart/chartspacefragment.cxx : -mrModel.mbAutoTitleDel = rAttribs.getBool( XML_val, !bMSO2007Document ); +mrModel.mbAutoTitleDel = rAttribs.getBool( XML_val, false ); and oox/source/drawingml/chart/chartspacemodel.cxx -mbAutoTitleDel( !bMSO2007Doc ), +mbAutoTitleDel( false ), The chart tile is correctly displayed. Is the !bMSO2007Document detection logic turned off or just not working correctly in this case?
(In reply to Luke from comment #13) > by changing oox/source/drawingml/chart/chartspacefragment.cxx : > -mrModel.mbAutoTitleDel = rAttribs.getBool( XML_val, !bMSO2007Document ); > +mrModel.mbAutoTitleDel = rAttribs.getBool( XML_val, false ); > > and oox/source/drawingml/chart/chartspacemodel.cxx > > -mbAutoTitleDel( !bMSO2007Doc ), > +mbAutoTitleDel( false ), > > The chart tile is correctly displayed. Is the !bMSO2007Document detection > logic turned off or just not working correctly in this case? The conditions are correct as can be seen by the unit test. Also a quick look into the spec, oart1 21.2.2.7 shows that the default value in OOXML is true. Of course if your document is not detected as MSO2007 which might not be the case for DOCX then the code will still use the OOXML values. So best to create a new bug report for the problem that DOCX 2007 documents are recognized early enough as MSO 2007 documents.
(In reply to Markus Mohrhard from comment #14) > The conditions are correct as can be seen by the unit test. Also a quick > look into the spec, oart1 21.2.2.7 shows that the default value in OOXML is > true. Could you point to the unit test, as it wasnt in any of the 3 commits in your blog post. > Of course if your document is not detected as MSO2007 which might not be the > case for DOCX then the code will still use the OOXML values. So best to > create a new bug report for the problem that DOCX 2007 documents are > recognized early enough as MSO 2007 documents. It should be noted that the chart app seems to be exporting MSO2007 charts, as reopening an LO saved xlsx in LO shows no chart title (bug 90530).
(In reply to Jay Philips from comment #15) > (In reply to Markus Mohrhard from comment #14) > > The conditions are correct as can be seen by the unit test. Also a quick > > look into the spec, oart1 21.2.2.7 shows that the default value in OOXML is > > true. > > Could you point to the unit test, as it wasnt in any of the 3 commits in > your blog post. http://cgit.freedesktop.org/libreoffice/core/commit/chart2/qa?id=e6dbd5b03cf923fa505f8313fbae56f2d287be30 > > > Of course if your document is not detected as MSO2007 which might not be the > > case for DOCX then the code will still use the OOXML values. So best to > > create a new bug report for the problem that DOCX 2007 documents are > > recognized early enough as MSO 2007 documents. > > It should be noted that the chart app seems to be exporting MSO2007 charts, > as reopening an LO saved xlsx in LO shows no chart title (bug 90530). The export code as a huge part of the import code are a mix of MSO 2007 and OOXML export. We are slowly moving to a pure OOXML export and an import that handles both.
(In reply to Markus Mohrhard from comment #16) > The export code as a huge part of the import code are a mix of MSO 2007 and > OOXML export. We are slowly moving to a pure OOXML export and an import that > handles both. Thanks for the info and the link.
Jay Philips Is your attachment 104062 [details] a native 2007, 2010 or LO export? If it's 2007, Markus wants a new bug report that 2007 DOCX documents are not recognized early enough as MSO 2007 documents. If it's a LO export, then we need a bug report that exported .DOCX files are recognized as 2007 instead of strict OOXML. (DOCX analog to Bug 90530) If it's 2010, I give up :)
(In reply to Luke from comment #18) > Is your attachment 104062 [details] a native 2007, 2010 or LO export? If > it's 2007, Markus wants a new bug report that 2007 DOCX documents are not > recognized early enough as MSO 2007 documents. I had tested the doc when i was triaging bug 82182, so i'm not 100% of its origins, but looking into app.xml, it says the doc was modified for 2-3 minutes in word 2007. > If it's a LO export, then we need a bug report that exported .DOCX files are > recognized as 2007 instead of strict OOXML. (DOCX analog to Bug 90530) Definitely not an LO export as synerzip does roundtrip work from MS to LO and then back.
Markus Mohrhard committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=914b4fb5a3c159bfd7cd8be2639185144f11d7ed import chart MSO 2007 streams correctly for docx files, tdf#82216 It will be available in 5.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.
*** Bug 89002 has been marked as a duplicate of this bug. ***
Migrating Whiteboard tags to Keywords: (bibisected, filter:docx) [NinjaEdit]
verified chart title 'Chart Title' is visible, chart is not visibleVersion: 5.5.0.0.alpha0+ Build ID: 066665644b398a882e6cded98af5bb060af41d76 TinderBox: Android-ARM@24-Bytemark-Hosting, Branch: Master, Time: 2017-06-01 00:30:43