Re: ppt import(?) bug hunting
* Caol?n McNamara (caol...@redhat.com) wrote: > On Sun, 2020-09-13 at 17:58 +0100, Dr. David Alan Gilbert wrote: > > > OK great, I'll look at sending a fix over the weekend. > > > > Done; https://gerrit.libreoffice.org/c/core/+/102542/1 > > > > https://gerrit.libreoffice.org/c/core/+/102590/1 > > looks good to me, merged now, thanks for that Thanks! Dave > ___ > LibreOffice mailing list > LibreOffice@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/libreoffice -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/ ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: ppt import(?) bug hunting
On Sun, 2020-09-13 at 17:58 +0100, Dr. David Alan Gilbert wrote: > > OK great, I'll look at sending a fix over the weekend. > > Done; https://gerrit.libreoffice.org/c/core/+/102542/1 > > https://gerrit.libreoffice.org/c/core/+/102590/1 looks good to me, merged now, thanks for that ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: ppt import(?) bug hunting
* Dr. David Alan Gilbert (d...@treblig.org) wrote: > * Caol?n McNamara (caol...@redhat.com) wrote: > > On Sun, 2020-09-06 at 16:30 +0100, Dr. David Alan Gilbert wrote: > > > So I'm now even more convinced that the right thing to do is just > > > nuke these two lines > > > > That does look convincing, that it was left over and accidentally not > > updated at 8a64144fddde61dd050da4cb93b4a7242a495bb2 to the new reality > > OK great, I'll look at sending a fix over the weekend. Done; https://gerrit.libreoffice.org/c/core/+/102542/1 > > > suggest how else I should test it? > > > > Along with your change to fix this, the ideal thing is to also add a > > test to (probably) sd/qa/unit/import-tests.cxx to assert the desired > > value for the numbering level > > I guess the fun is trying to find the right data - maybe like testTdf90626? that ends up as: https://gerrit.libreoffice.org/c/core/+/102590/1 Dave > Dave > > > > > ___ > > LibreOffice mailing list > > LibreOffice@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/libreoffice > -- > -Open up your eyes, open up your mind, open up your code --- > / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ > \dave @ treblig.org | | In Hex / > \ _|_ http://www.treblig.org |___/ > ___ > LibreOffice mailing list > LibreOffice@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/libreoffice -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/ ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: ppt import(?) bug hunting
* Caol?n McNamara (caol...@redhat.com) wrote: > On Sun, 2020-09-06 at 16:30 +0100, Dr. David Alan Gilbert wrote: > > So I'm now even more convinced that the right thing to do is just > > nuke these two lines > > That does look convincing, that it was left over and accidentally not > updated at 8a64144fddde61dd050da4cb93b4a7242a495bb2 to the new reality OK great, I'll look at sending a fix over the weekend. > > suggest how else I should test it? > > Along with your change to fix this, the ideal thing is to also add a > test to (probably) sd/qa/unit/import-tests.cxx to assert the desired > value for the numbering level I guess the fun is trying to find the right data - maybe like testTdf90626? Dave > > ___ > LibreOffice mailing list > LibreOffice@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/libreoffice -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/ ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: ppt import(?) bug hunting
On Sun, 2020-09-06 at 16:30 +0100, Dr. David Alan Gilbert wrote: > So I'm now even more convinced that the right thing to do is just > nuke these two lines That does look convincing, that it was left over and accidentally not updated at 8a64144fddde61dd050da4cb93b4a7242a495bb2 to the new reality > suggest how else I should test it? Along with your change to fix this, the ideal thing is to also add a test to (probably) sd/qa/unit/import-tests.cxx to assert the desired value for the numbering level ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: ppt import(?) bug hunting
* Dr. David Alan Gilbert (d...@treblig.org) wrote: > * Dennis Roczek (dennisroc...@libreoffice.org) wrote: > > Hi Dave, > > > > Am 04.09.2020 um 16:48 schrieb Dr. David Alan Gilbert: > > > * Dennis Roczek (dennisroc...@libreoffice.org) wrote: > > >> Hi Dave, > > > > > > Thanks for the reply, > > > > > >> Am 04.09.2020 um 02:52 schrieb Dr. David Alan Gilbert: > > >>> That code predates the move of the msfilters into that directory - > > >>> is there a tree with history older than that still around so I can > > >>> find the bug/history/reasoning of where that pair of lines came from? > > >> > > >> Yep, there is: > > >> https://cgit.freedesktop.org/libreoffice/filters/tree/filter/source/msfilter?h=libreoffice-3-3-4 > > >> > > >> (Branch might need to be adjusted) > > > > > > Thanks; I'm not sure that goes back far enough - I see commit 6e07a590d4 > > > in there, which pulls the code in and it's titled: > > > > > > '#i106421#: move msfilter to filter' > > and that is > > https://bz.apache.org/ooo/show_bug.cgi?id=106421 > > > > Wow, you went back in time to 2009... > > New bugs are boring - this bug goes back a bit > before that, at least to: > https://bz.apache.org/ooo/show_bug.cgi?id=99843 > in OOo 3.0.1 > > > > and it's all additions, no removals or moves; > > > so it's pulling the code in from somewhere else. > > Yep. You're back in core (sd/source/filter/eppt/) now. > > I'm not sure if it is that - this is the import code > and there seem to (currently) be sd/source/filter/ppt/pptin.cxx > and filter/source/msfilter/svdfppt.cxx - it's the svdfppt.cxx > the code is currently in. I think I've found the cause of this problem, I've not found the original source of this line of code, but I see the commit that broke it. That overwrite of level 0 has been there at least as far back as 1.1.1-4 in 2004; if ( eNumRuleType == SVX_RULETYPE_PRESENTATION_NUMBERING ) aRule.SetLevel( 0, aNumberFormat ); but the idea of the depth changed in https://bz.apache.org/ooo/show_bug.cgi?id=75927 commit 8a64144fddde61dd050da4cb93b4a7242a495bb2 Author: Rüdiger Timm Date: Fri Jun 6 11:34:05 2008 + INTEGRATION: CWS impressodf12 (1.158.12); FILE MERGED 2008/06/04 14:29:44 sj 1.158.12.8: #i90357# fixed bullet indentation when bullets are turned off 2008/05/30 11:59:21 cl 1.158.12.7: fixed unix compile errors 2008/05/30 09:36:37 sj 1.158.12.6: #i75927# taking care of outliner core changes (numbering) 2008/05/29 11:35:40 sj 1.158.12.5: #i75927# taking care of outliner core changes (numbering) 2008/05/26 11:43:09 cl 1.158.12.4: #i35937# code cleanup after bullet rework 2008/04/25 09:00:00 cl 1.158.12.3: RESYNC: (1.158-1.159); FILE MERGED 2008/04/24 15:30:07 cl 1.158.12.2: #i35937# converted EE_PARA_BULLETSTATE to bool item 2008/04/10 16:50:56 cl 1.158.12.1: #i35937# allow paragraph depth of -1 to switch of numbering https://bz.apache.org/ooo/show_bug.cgi?id=75927 the way this code used to work before that commit was that it read in data from the wire into slots 1..4 and then duped them from 5..9 (or was it 10) Then those two lines filled in the empty slot 0. the code used to have: case TSS_TYPE_BODY : case TSS_TYPE_HALFBODY : case TSS_TYPE_QUARTERBODY : nDepth = 1; nLevels = 9; eNumRuleType = SVX_RULETYPE_PRESENTATION_NUMBERING; which is what started reading it at 0. Now in that commit from June 2008 (somewhere just before OOo 3.0.0) it looks like the outliner code changed how the numbering worked (see that bz 75927), and it has: case TSS_TYPE_BODY : case TSS_TYPE_HALFBODY : case TSS_TYPE_QUARTERBODY : -nDepth = 1; -nLevels = 9; +nLevels = 10; so it starts reading straight into slot 0 - but it leaves in my favourite two lines above that nuke slot 0, which now isn't really the same slot 0 as it was previously. So I'm now even more convinced that the right thing to do is just nuke these two lines; does anyone understand this code better before I do this, or suggest how else I should test it? Dave > > I cannot remember if we merged all history from hg, svn, cws to git... > > > > If not, somebody else (Thorsten? *g*) has to help as these old > > repositories are no longer online, if I remember correctly. > > Would be nice! > > Dave > > > Best > > > > Dennis > > > > > > -- > -Open up your eyes, open up your mind, open up your code --- > / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ > \dave @ treblig.org | | In Hex / > \ _|_ http://www.treblig.org |___/ > ___ > LibreOffice mailing list > LibreOffice@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/libreoffice -- -Open up your eyes, open up your
Re: ppt import(?) bug hunting
Hi Dave, Am 04.09.2020 um 16:48 schrieb Dr. David Alan Gilbert: > * Dennis Roczek (dennisroc...@libreoffice.org) wrote: >> Hi Dave, > > Thanks for the reply, > >> Am 04.09.2020 um 02:52 schrieb Dr. David Alan Gilbert: >>> That code predates the move of the msfilters into that directory - >>> is there a tree with history older than that still around so I can >>> find the bug/history/reasoning of where that pair of lines came from? >> >> Yep, there is: >> https://cgit.freedesktop.org/libreoffice/filters/tree/filter/source/msfilter?h=libreoffice-3-3-4 >> >> (Branch might need to be adjusted) > > Thanks; I'm not sure that goes back far enough - I see commit 6e07a590d4 > in there, which pulls the code in and it's titled: > > '#i106421#: move msfilter to filter' and that is https://bz.apache.org/ooo/show_bug.cgi?id=106421 Wow, you went back in time to 2009... > and it's all additions, no removals or moves; > so it's pulling the code in from somewhere else. Yep. You're back in core (sd/source/filter/eppt/) now. I cannot remember if we merged all history from hg, svn, cws to git... If not, somebody else (Thorsten? *g*) has to help as these old repositories are no longer online, if I remember correctly. Best Dennis signature.asc Description: OpenPGP digital signature ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: ppt import(?) bug hunting
* Dennis Roczek (dennisroc...@libreoffice.org) wrote: > Hi Dave, > > Am 04.09.2020 um 16:48 schrieb Dr. David Alan Gilbert: > > * Dennis Roczek (dennisroc...@libreoffice.org) wrote: > >> Hi Dave, > > > > Thanks for the reply, > > > >> Am 04.09.2020 um 02:52 schrieb Dr. David Alan Gilbert: > >>> That code predates the move of the msfilters into that directory - > >>> is there a tree with history older than that still around so I can > >>> find the bug/history/reasoning of where that pair of lines came from? > >> > >> Yep, there is: > >> https://cgit.freedesktop.org/libreoffice/filters/tree/filter/source/msfilter?h=libreoffice-3-3-4 > >> > >> (Branch might need to be adjusted) > > > > Thanks; I'm not sure that goes back far enough - I see commit 6e07a590d4 > > in there, which pulls the code in and it's titled: > > > > '#i106421#: move msfilter to filter' > and that is > https://bz.apache.org/ooo/show_bug.cgi?id=106421 > > Wow, you went back in time to 2009... New bugs are boring - this bug goes back a bit before that, at least to: https://bz.apache.org/ooo/show_bug.cgi?id=99843 in OOo 3.0.1 > > and it's all additions, no removals or moves; > > so it's pulling the code in from somewhere else. > Yep. You're back in core (sd/source/filter/eppt/) now. I'm not sure if it is that - this is the import code and there seem to (currently) be sd/source/filter/ppt/pptin.cxx and filter/source/msfilter/svdfppt.cxx - it's the svdfppt.cxx the code is currently in. > I cannot remember if we merged all history from hg, svn, cws to git... > > If not, somebody else (Thorsten? *g*) has to help as these old > repositories are no longer online, if I remember correctly. Would be nice! Dave > Best > > Dennis > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/ ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: ppt import(?) bug hunting
* Dennis Roczek (dennisroc...@libreoffice.org) wrote: > Hi Dave, Thanks for the reply, > Am 04.09.2020 um 02:52 schrieb Dr. David Alan Gilbert: > > That code predates the move of the msfilters into that directory - > > is there a tree with history older than that still around so I can > > find the bug/history/reasoning of where that pair of lines came from? > > Yep, there is: > https://cgit.freedesktop.org/libreoffice/filters/tree/filter/source/msfilter?h=libreoffice-3-3-4 > > (Branch might need to be adjusted) Thanks; I'm not sure that goes back far enough - I see commit 6e07a590d4 in there, which pulls the code in and it's titled: '#i106421#: move msfilter to filter' and it's all additions, no removals or moves; so it's pulling the code in from somewhere else. Dave > Best regards, > > Dennis Roczek > -- -Open up your eyes, open up your mind, open up your code --- / Dr. David Alan Gilbert| Running GNU/Linux | Happy \ \dave @ treblig.org | | In Hex / \ _|_ http://www.treblig.org |___/ ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: ppt import(?) bug hunting
Hi Dave, Am 04.09.2020 um 02:52 schrieb Dr. David Alan Gilbert: > That code predates the move of the msfilters into that directory - > is there a tree with history older than that still around so I can > find the bug/history/reasoning of where that pair of lines came from? Yep, there is: https://cgit.freedesktop.org/libreoffice/filters/tree/filter/source/msfilter?h=libreoffice-3-3-4 (Branch might need to be adjusted) Best regards, Dennis Roczek signature.asc Description: OpenPGP digital signature ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice