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
ppt import(?) bug hunting
Hi,
I've been poking about trying to understand a pet bug:
https://bugs.documentfoundation.org/show_bug.cgi?id=49856
which is related to bullets in documents that go through
a ppt export->import cycle; I've not yet quite convinced
myself of whether it's an export or import bug, but I'm
down to a couple of lines of code that look suspicious
but I could do with a hand understanding why they exist.
In the test case, you end up where shift-tab'ing the victim
line ends up indenting and switching to a >> bullet rather
than doing a simple unindent and getting a round-blob bullet.
I've followed the import code, found a situation where the
number formats had that >> bullet in the first slot and followed
it back, and got as far as svdfppt.cxx, PPTStyleSheet::PPTStyleSheet
which, near the bottom has:
for ( sal_uInt16 nCount = 0; nDepth < nLevels; nCount++ )
{
const PPTParaLevel& rParaLevel = mpParaSheet[ i ]->maParaLevel[
nCount ];
const PPTCharLevel& rCharLevel = mpCharSheet[ i ]->maCharLevel[
nCount ];
SvxNumberFormat aNumberFormat( SVX_NUM_CHAR_SPECIAL );
aNumberFormat.SetBulletChar( ' ' );
GetNumberFormat( rManager, aNumberFormat, nCount, rParaLevel,
rCharLevel, i );
aRule.SetLevel( nDepth++, aNumberFormat );
if ( nCount >= 4 )
{
for ( ;nDepth < nLevels; nDepth++ )
aRule.SetLevel( nDepth, aNumberFormat );
if ( eNumRuleType == SvxNumRuleType::PRESENTATION_NUMBERING )
aRule.SetLevel( 0, aNumberFormat );
}
}
It seems to be that rule that's overwriting the 0th level with
the 4th (?) level - but I don't understand what that line
is intended to do or why it's needed.
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?
If I remove those 2 lines then my bug disappears - but I'm loathed
to submit a patch to do that without understanding why they are there
in the first place.
(The variable usage in general in that loop is a little odd to me;
there's nCount and nDepth but I don't see a situation in which they
diverge).
I'm also a bit confused why this doesn't cause a bigger problem, it doesn't
seem to be level 0 is always wrong, but I've not figured out why.
Thanks in advance,
Dave
--
-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
