[Kde-pim] Re: Review Request: kmail: now it is allowed to create a new folder on the top of an imap folder

[Antonis Tsiapaliokas]

> On July 24, 2011, 8:35 a.m., Volker Krause wrote:
> > resources/imap/retrievecollectionstask.cpp, line 175
> > <http://git.reviewboard.kde.org/r/102031/diff/1/?file=27782#file27782line175>
> >
> >     Why this change? \NoSelect is set on structural folders that don't physically exist, you can't really create sub-folders of those in general.

So you mean that this changes doesn't belong to the imap folder, but it belongs to a sub-folder of the imap?


> On July 24, 2011, 8:35 a.m., Volker Krause wrote:
> > resources/imap/retrievecollectionstask.cpp, line 50
> > <http://git.reviewboard.kde.org/r/102031/diff/1/?file=27782#file27782line50>
> >
> >     I think this should be the only really needed change for this. However, I'm unsure what side-effects this has on IMAP servers that have only INBOX at top-level and everything else below that (e.g. Cyrus).

Why INBOX will be the top-level folder? I thought that kmail was creating a virtual top-level folder for each imap account....


> On July 24, 2011, 8:35 a.m., Volker Krause wrote:
> > resources/imap/retrievecollectionstask.cpp, line 49
> > <http://git.reviewboard.kde.org/r/102031/diff/1/?file=27782#file27782line49>
> >
> >     This is wrong nevertheless, the root folder cannot contain mails, so it should not claim it does. Besides, isStructural() should only return true for empty content mime type lists, which is not the case here, it contains Collection::mimeType().

Yes, but without that the "Add folder" actions is disable in kmail... So how does it sound to you, if we add a new function in kmmainwidget.cpp, which will change that only when we are selecting an imap folder and when we are not then the contentMimeType will become again empty...


> On July 24, 2011, 8:35 a.m., Volker Krause wrote:
> > resources/imap/addcollectiontask.cpp, line 53
> > <http://git.reviewboard.kde.org/r/102031/diff/1/?file=27780#file27780line53>
> >
> >     KJob::exec() is a no-go in resources due to the reentrancy problems it introduces. But why do you need a folder separator for top-level folders at all?

ok, then we will use signals and slots. Because if the separator is different that '/', then if you try to check the mails, the local folders are deleted and the kmail is crashing.


> On July 24, 2011, 8:35 a.m., Volker Krause wrote:
> > resources/imap/retrievecollectionmetadatatask.cpp, line 225
> > <http://git.reviewboard.kde.org/r/102031/diff/1/?file=27781#file27781line225>
> >
> >     This allows creation of folders in all collections, even shared read-only folders of other users or those marked as \NoSelect, no?

I think in all collections... So i guess that it should go... I have not thought that...


- Antonis


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102031/#review5020
-----------------------------------------------------------


On July 21, 2011, 4:46 p.m., Antonis Tsiapaliokas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102031/
> -----------------------------------------------------------
> 
> (Updated July 21, 2011, 4:46 p.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Summary
> -------
> 
> Hello
> 
> On kmail 2, if you try to add a folder under your top imap folder, you will see that the button "Add folder" is disable. This patch is fixing this issue.
> 
> 
> This addresses bug 251704.
>     http://bugs.kde.org/show_bug.cgi?id=251704
> 
> 
> Diffs
> -----
> 
>   resources/imap/addcollectiontask.cpp 5ce6aec 
>   resources/imap/retrievecollectionmetadatatask.cpp 3c9ead4 
>   resources/imap/retrievecollectionstask.cpp 48f0e57 
> 
> Diff: http://git.reviewboard.kde.org/r/102031/diff
> 
> 
> Testing
> -------
> 
> Kdepim and kdepim-runtime compiles and runs without any issue.
> 
> 
> Thanks,
> 
> Antonis
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/