[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kmail-devel
Subject:    Re: [PATCH] fix non latin1 folder name handling
From:       Ingo =?iso-8859-1?q?Kl=F6cker?= <kloecker () kde ! org>
Date:       2004-02-29 20:23:52
Message-ID: 200402292123.52594 () erwin ! ingo-kloecker ! de
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


On Saturday 28 February 2004 19:51, Toshitaka Fujioka wrote:
> Hello,
>
> Current CVS HEAD's kmail can not use non latin1 folder name,
> correctly. eg. dose not work file in to folder filter action
>
> The attached patch is fix this problem.
> Please review.

Thanks for the patch, but...

> I'll commit this patch, if anyone has no objection.

Objection. ;-) See below.

> Index: folderstorage.cpp
> ===================================================================
> RCS file: /home/kde/kdepim/kmail/folderstorage.cpp,v
> retrieving revision 1.13
> diff -u -3 -d -p -r1.13 folderstorage.cpp
> --- folderstorage.cpp   26 Feb 2004 11:56:32 -0000      1.13
> +++ folderstorage.cpp   28 Feb 2004 18:31:32 -0000
> @@ -709,7 +709,7 @@ QString FolderStorage::label() const
>    if ( folder() && folder()->isSystemFolder() && !mLabel.isEmpty() )
>       return mLabel;
>    if ( folder() && folder()->isSystemFolder() )
> -     return i18n( folder()->name().latin1() );
> +    return folder()->name();
>    return mFolder->name();
>  }

This is wrong. The API docu of KMFolder::label() says "Returns the label 
of the folder for visualization.". And 'for visualization' means 
translated. OTOH, I don't think FolderStorage::label() should even 
exist. The label definitely belongs to the folder, but not to the 
storage.

If FolderStorage::label() is used somewhere although 
FolderStorage::name() should be used then those calls have to be fixed.

Also I wonder what this part of the patch should fix. The filenames of 
all system folders are in Latin 1 (actually they are even in ASCII).

=====

> -  QString escapedName = QString( name() );
> +  QString escapedName = QString::fromLocal8Bit( name() );

I'm pretty sure that the correct fix is the following:

> -  QString escapedName = QString( name() );
> +  QString escapedName = QString( folder()->name() );

This seems to be just another folderstorage regression.

=====

> -    mStorage = new KMFolderCachedImap( this, aFolderName.latin1() );
> +    mStorage = new KMFolderCachedImap( this, 
> aFolderName.local8Bit().data() ); 

This part of the patch and the other similar changes shouldn't hurt. But 
it also shouldn't really change anything because aFolderName.latin1() 
or aFolderName.local8Bit().data() is only used of name for the 
FolderStorage-QObject. But this name is pretty much irrelevant because 
it shouldn't be used anywhere.

=====

All in all I guess that my proposed patch
> -  QString escapedName = QString( name() );
> +  QString escapedName = QString( folder()->name() );
in FolderStorage::idString() will fix the problem. Till agrees that 
name() is wrong, so I'll commit this change. Please try if this fixes 
your problems.

Regards,
Ingo

[Attachment #5 (application/pgp-signature)]

_______________________________________________
KMail developers mailing list
KMail-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmail-devel


[prev in list] [next in list] [prev in thread] [next in thread]