Project JEDI - Issue Tracker
Mantis Bugtracker

Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0005917 [JEDI VCL] 00 JVCL Components crash always 2012-06-26 11:44 2015-09-21 17:47
Reporter bhe View Status public  
Assigned To obones
Priority normal Resolution fixed  
Status resolved   Product Version Daily / GIT
Summary 0005917: JvGnugettext TranslateStrings Clear has side effects
Description TranslateStrings cares about OwnsObjects but sl.Clear throws all objects away anyway.!

Replacing
          sl.Clear;
          sl.AddStrings(s);
with
          for i := 0 to sl.Count - 1 do
            sl.Strings[i] := s[i];

work well.
This way the OwnsObjects is not required at all.
Additional Information (It looks bit like the return of http://issuetracker.delphi-jedi.org/view.php?id=4273) [^]
Tags No tags attached.
Attached Files

- Relationships

-  Notes
(0020224)
bhe (reporter)
2012-09-26 13:57

Still not fixed in 3.47! Why?

Thus JvGnugettext is not usable in real world apps:
E.g. not with DevExpress Bars:

TdxBarManagerCategories = class(TStringList)
...
procedure TdxBarManagerCategories.Clear;
begin
  ClearObjects; <- Fatal
  inherited;
end;

Calling Clear for that private class is not provided!

My suggested solution should have less (likely no) side effects.
(0020225)
AHUser (developer)
2012-09-26 18:52

> Still not fixed in 3.47! Why?

Because nobody had a look at this. There are not many left who work on the JCL and JVCL code and those who are, don't spend their whole spare time on it.



> My suggested solution should have less (likely no) side effects.

Your changes have the effect that you can end up with a messed up translation for a sorted string list (assigning a translated string to Strings[] changes the order of the strings in the list).

I have now added code to set all sl.Objects[] to nil before calling sl.Clear. That should fix the bug and not introduce the "sorted string list" issue.

The svn revision is 13449. I leave that report open so you can add comment it (in case that fix doesn't help you)
(0020228)
bhe (reporter)
2012-09-26 23:41

Why not temporary setting Sorted to False?
Or special dealing for sorted and unsorted lists?

"Niling" the Objects might work (likely it will in the sample case).
But you never know what happens in an derived class
if PutObject() is overridden.

Btw. auto translation of a sorted string list in a persistent component
likely will not work in practical cases.
Maybe because of this i had no issues with my solution.
(0020229)
AHUser (developer)
2012-09-27 09:49

The older Mantis issue 4273 explicitly tests for the ClassType. Maybe we should restore that code, that got lost when the JvGnuGetText was synchronized with the dxgettext (http://dxgettext.po.dk/) [^] code. The older code also just ignored the Sorted property for "unknown" ClassTypes.

      if (sl.ClassType = TStrings) or (sl.ClassType = TStringList) or
         (sl.ClassName = 'TMemoStrings') or (sl.ClassName = 'TComboBoxStrings') or
         (sl.ClassName = 'TListBoxStrings') then
(0020235)
bhe (reporter)
2012-09-27 12:49

That could be a way to go - at least i had no issue with that implementation.

Just ignoring the Sorted prop can lead to an EStringListError(SSortedListError).
Perhaps a dedicated assertion for this case could be a good indication for those who using GetText in such a questionable way.
(0020270)
bhe (reporter)
2012-10-26 01:37

Their is a small "typo" in:

          if (sl.ClassType <> TStringList) and (sl is TStringList) then
            for I := 0 to sl.Count do
              sl.Objects[I] := nil;
Has to be:
          if (sl.ClassType <> TStringList) and (sl is TStringList) then
            for I := 0 to sl.Count - 1 do
              sl.Objects[I] := nil;

Something happens to You!? ;-)

Besides that, the initial scenario work with that solution!
So far so good.

Nevertheless i prefer my simple assign approach for unsorted list:

          for i := 0 to sl.Count - 1 do
            sl[i] := s[i];

Thus avoids calls to Clear and AddStrings - who knows how they are overwritten. Should be slightly faster too.
(0020336)
obones (administrator)
2013-01-15 15:01

Could you look at the publicly available gnugettext to see how they do it? I'm quite sure there were fixes in it that we did not incorporate back
(0020401)
bhe (reporter)
2013-01-16 14:01

You mean http://dxgettext.svn.sourceforge.net/viewvc/dxgettext/trunk/dxgettext? [^]

They just care about OwnsObjects but not about Sorted.
(Which was never an issue for me - see statement from 2012-09-26 23:41)

But they use Clear + AddStrings which fails!

My productive code look like that {Comments removed):

procedure TGnuGettextInstance.TranslateStrings(sl: TStrings;const TextDomain:DomainString);
...
        {$ifdef DELPHI2009OROLDER}
        if Assigned(slAsTStringList) then begin
          originalOwnsObjects := slAsTStringList.OwnsObjects;
          slAsTStringList.OwnsObjects := False;
        end;
        {$endif}
        try
          if(sl is TStringList)and TStringList(sl).Sorted then
          begin
            if (sl.ClassType <> TStringList) and (sl is TStringList) then
              for I := 0 to sl.Count - 1 do
                sl.Objects[I] := nil;
            sl.Clear;
            sl.AddStrings(s);
          end else
            for i := 0 to sl.Count - 1 do
              sl[i] := s[i];
        finally
          {$ifdef DELPHI2009OROLDER}
          if Assigned(slAsTStringList) then
            slAsTStringList.OwnsObjects := originalOwnsObjects;
          {$endif}
        end;

(Used in several successful applications. One of them with 1.3 Mio LoC
got more than 1 Mio downloads and is running in 13 languages.)
(0020867)
obones (administrator)
2013-12-16 17:03

I'm confused.
You say that Clear + AddStrings fails when Sorted is True, and yet, your "production" code calls Clear + AddStrings when the list is sorted.
Which one is the correct one?
(0020871)
bhe (reporter)
2013-12-17 00:38

Your are right - but AFTER setting all objects to Nil.
Here a commented version:

if(sl is TStringList)and TStringList(sl).Sorted then
begin
  // Classes inheriting from TStringList might free objects in overriden Clear()
  // Thus we remove all object before calling Clear()
  if (sl.ClassType <> TStringList) and (sl is TStringList) then
    for I := 0 to sl.Count - 1 do
      sl.Objects[I] := nil;
  sl.Clear;
  sl.AddStrings(s); // This restores Objects too!
end else
  for i := 0 to sl.Count - 1 do
    sl[i] := s[i];
(0020880)
obones (administrator)
2013-12-18 15:28

Ok, fair enough, but when the string list is not sorted, this means we use the "sl[i] := s[i]" construct, and thus we loose the objects that AddStrings would have restored.
(0020889)
bhe (reporter)
2013-12-18 17:29

Why? TStringList.Put() does not effect the objects.

Btw. the OwnsObjects handling is IMHO only required in the Sorted case.
OwnsObjects (in TStringList) only matters for Clear, Delete and Destroy.
(0020891)
obones (administrator)
2013-12-24 16:41

Yes, you are right.
This is now in GIT.

- Issue History
Date Modified Username Field Change
2012-06-26 11:44 bhe New Issue
2012-09-26 13:49 bhe Issue Monitored: bhe
2012-09-26 13:57 bhe Note Added: 0020224
2012-09-26 18:52 AHUser Note Added: 0020225
2012-09-26 18:52 AHUser Status new => feedback
2012-09-26 23:41 bhe Note Added: 0020228
2012-09-27 09:49 AHUser Note Added: 0020229
2012-09-27 12:49 bhe Note Added: 0020235
2012-10-26 01:37 bhe Note Added: 0020270
2013-01-15 15:01 obones Note Added: 0020336
2013-01-16 14:01 bhe Note Added: 0020401
2013-12-13 11:40 obones Status feedback => acknowledged
2013-12-16 17:03 obones Note Added: 0020867
2013-12-16 17:03 obones Status acknowledged => feedback
2013-12-17 00:38 bhe Note Added: 0020871
2013-12-18 15:28 obones Note Added: 0020880
2013-12-18 17:29 bhe Note Added: 0020889
2013-12-24 16:41 obones Note Added: 0020891
2013-12-24 16:41 obones Status feedback => resolved
2013-12-24 16:41 obones Fixed in Version => Daily / GIT
2013-12-24 16:41 obones Resolution open => fixed
2013-12-24 16:41 obones Assigned To => obones
2015-09-21 17:47 obones Fixed in Version Daily / GIT => 3.49


Mantis 1.1.6[^]
Copyright © 2000 - 2008 Mantis Group
Powered by Mantis Bugtracker