Created attachment 76259 [details] Document ilustrating wrong Mid statement behaviour Problem description: In LibreOffice 3, for this Basic code: s = "The lightbrown fox" Mid(s,5,10,"lazy") the resulting string was "The lazy fox". In LibreOffice 4, string replacement isn't done correctly and works ONLY if replacement string is long EXACTLY as the part it replaces. Steps to reproduce: 1. Try included ods file Current behavior: Mid statement replacement produces string with wong characters Expected behavior: Mid statement should replace string as in LibreOffice 3 Operating System: Ubuntu Version: 4.0.1.1 rc Last worked in: 3.5.4 release
Created attachment 76260 [details] The result of using Mid(s,5,10,"lazy")
updating version as you've said last working version was 3.5.4 release. We use version field to show first version we see problem, not latest it's been tested on (we use comments to say we've tested on newer version and see same problem) Also this will not constitute a major bug (no loss of data, no crash, no memory issues). This is just a normal bug. Marking as NORMAL. Not confirming the issue as I want to see what it looks like using the last version it worked in order to see the difference.
Additional note for any QA personnel - I tried to bibisect this but see exact same behavior in earliest bibisect as I do with 4.0.0.3 release as well as 4.1 master :-/
I'm not sure that 3.5.4 was last working version, but it was last form 3.x that I used.
On pc Debian x86-64 with master sources updated yesterday, I reproduced the problem. Noel: one for you?
Noel, what about this patch? I tested it for this case and it works but perhaps I forgot something. diff --git a/basic/source/runtime/methods.cxx b/basic/source/runtime/methods.cxx index 5dc45ed..12dfff2 100644 --- a/basic/source/runtime/methods.cxx +++ b/basic/source/runtime/methods.cxx @@ -1266,7 +1266,7 @@ RTLFUNC(Mid) { aResultStr = aArgStr; aResultStr.remove( nStartPos, nLen ); - aResultStr.insert( nStartPos, rPar.Get(4)->GetOUString().getStr(), nLen); + aResultStr.insert( nStartPos, rPar.Get(4)->GetOUString().getStr(), std::min(nLen, rPar.Get(4)->GetOUString().getLength())); } rPar.Get(1)->PutString( aResultStr.makeStringAndClear() );
Julien - do you see this with 3.6? If not, I must be misunderstanding something, I'd like to get a bibisect if possible
(In reply to comment #7) > Julien - do you see this with 3.6? If not, I must be misunderstanding > something, I'd like to get a bibisect if possible Sorry Joel, I don't have anymore 3.6 or 4.0 sources (I focus on master sources). However I tested on 3.5.4 (Debian package version) and I didn't reproduce the bug.
Also, I think this block should be common (before the if bCompatibility) sal_Int32 nArgLen = aArgStr.getLength(); if( nStartPos + 1 > nArgLen ) { StarBASIC::Error( SbERR_BAD_ARGUMENT ); return; } Because if not, Mid(s, 20, 10, "lazy") will have the same kind of effect.
(In reply to comment #5) > On pc Debian x86-64 with master sources updated yesterday, I reproduced the > problem. > > Noel: one for you? I guess so, no idea what changed here, presumeably either aoo resync or String->OUString work 'cause I don't recall any other changes to this stuff since Libreoffice came into existence ( could be wrong about that though )
(In reply to comment #6) > Noel, what about this patch? I tested it for this case and it works but > perhaps I forgot something. > > diff --git a/basic/source/runtime/methods.cxx > b/basic/source/runtime/methods.cxx > index 5dc45ed..12dfff2 100644 > --- a/basic/source/runtime/methods.cxx > +++ b/basic/source/runtime/methods.cxx > @@ -1266,7 +1266,7 @@ RTLFUNC(Mid) > { > aResultStr = aArgStr; > aResultStr.remove( nStartPos, nLen ); > - aResultStr.insert( nStartPos, > rPar.Get(4)->GetOUString().getStr(), nLen); > + aResultStr.insert( nStartPos, > rPar.Get(4)->GetOUString().getStr(), std::min(nLen, > rPar.Get(4)->GetOUString().getLength())); > } > > rPar.Get(1)->PutString( aResultStr.makeStringAndClear() ); yes, this looks good ( 'cause the old UniString/String used autocorrect that )
(In reply to comment #9) > Also, I think this block should be common (before the if bCompatibility) > sal_Int32 nArgLen = aArgStr.getLength(); > if( nStartPos + 1 > nArgLen ) > { > StarBASIC::Error( SbERR_BAD_ARGUMENT ); > return; > } > Because if not, Mid(s, 20, 10, "lazy") will have the same kind of effect. I'd prefer not to change it just yet, I am not sure if changing it is correct, previously this scenario would result in the string being appended to the end of the string, that behaviour has now changed ( it's another silent correction that UniString/String used to do )
Julien Nabet committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=14d452d0c2874f47f0b6ac1cd1c725195d5d69a9 fix for fdo#62090 Mid function regression 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.
Noel Power committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=0054b438013ef412c2df3b7ebe93dd1935ec07af follow on fix for fdo#62090 ensure out of range startpos is handled 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.
Julien Nabet committed a patch related to this issue. It has been pushed to "libreoffice-4-0": http://cgit.freedesktop.org/libreoffice/core/commit/?id=69371d3fae481adb2b21701952570f829fcd8311&h=libreoffice-4-0 fix for fdo#62090 Mid function regression It will be available in LibreOffice 4.0.3. 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.
Noel Power committed a patch related to this issue. It has been pushed to "libreoffice-4-0": http://cgit.freedesktop.org/libreoffice/core/commit/?id=df4b1b9be4f818ef912bfb58b514bab1acdd6e71&h=libreoffice-4-0 follow on fix for fdo#62090 ensure out of range startpos is handled It will be available in LibreOffice 4.0.3. 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.
@Noel: Is this bug fixed?
(In reply to comment #17) > @Noel: Is this bug fixed? yup, should be
Version set from description. Best regards. JBF