[KPhotoAlbum] Strange behaviour of tagging dialog in hierarchical view

[Martin Jost]

Am 30.03.2013 16:19, schrieb Robert Krawitz:
> On Sat, 30 Mar 2013 15:58:57 +0100, Martin Jost wrote:
>> I just feel confused by the behaviour of KPA 4.4.

>> Now here is my current problem:
>> I had the KPA tagging dialog open and switched to tree view.
>> The current foto is untagged. I click "Freudenstadt" - and KPA not only
>> switches this on, but also all sub-categories below Freudenstadt
>> (Empfingen, Kniebis, Zollernstraße, ...)
>>
>> Problem 1:
>> This doesn't happen every time, but only sometimes. What is the "rule"
>> behind this ?
>>
>> Problem 2:
>> I consider this plain wrong. If the photo belongs to "Freudenstadt", it
>> doesn't belong to Empfingen and Kniebis and ...
>> (If the hierarchy would be
>> Europe
>>   Germany
>>   France
>>   Spain
>>   ...
>> Surely a photo taken somewhere in Europe isn't taken in Germany, France,
>> Spain, ... all at the same time.)
>>
>> Worse: If I would now add "Norway" below "Europe", KPA surely wouldn't
>> add "Norway" to all photos tagged "Europe" previously - only the newly
>> ones would get tagged this way.
>>
>> What is the reason/motivation behind this behaviour ?
> 
> I believe this is a bug that was fixed post-4.4, in commits
> 585656c14b60e280fad05bc9dde19771df807c5b (Feb. 19) and later slightly
> differently in 17e43680e78eb090947c6cfc9bf310486dffa12c (Mar. 6).

Hello,

sorry, please ignore - once more - my git-ignorance. (I'm used to CVS
and SVN).
I tried to apply these changes to my 4.4 copy.
Here is what I did:

Searched for the hashes: (First March 6)

> git log (and searching for the hash): (my emphasize)

> commit >>>>>>>>>>>>>>>>>>>>17e43680e78eb090947c6cfc9bf310486dffa12c<<<<<<<<<<<<<<<<<<<<<
> Author: Reimar Imhof <Reimar.Imhof at netcologne.de>
> Date:   Wed Mar 6 18:50:48 2013 +0200
> 
>     In configureItem I removed the setTristate-call because I've found the
>     following hint from
>     http://lists.trolltech.com/qt-interest/2008-04/thread00080-0.html: "The
>     intended usage of Qt::ItemIsTristate is to signify that the checked
>     state of an item should be computed as a function of the  checked states
>     of its child items; e.g. no child items checked =>  parent becomes
>     unchecked; some child items checked => parent is  partially checked; all
>     child items checked => parent item checked."
> 
> commit 24be3677d183dd3d21dd120361097cc6b8815f8a
> Author: Johannes Zarl <isilmendil at isilme.lupus.uberspace.de>
> Date:   Wed Feb 27 23:01:59 2013 +0100
> 
>     Make EXIF label search more obvious.

Get the diff:
>git diff 24be3677d183dd3d21dd120361097cc6b8815f8a
17e43680e78eb090947c6cfc9bf310486dffa12c

That should be the right "direction" for the diff - isn't it ?

> diff --git a/AnnotationDialog/ListSelect.cpp b/AnnotationDialog/ListSelect.cpp
> index 4db70a3..46d8129 100644
> --- a/AnnotationDialog/ListSelect.cpp
> +++ b/AnnotationDialog/ListSelect.cpp
> @@ -196,6 +196,8 @@ void AnnotationDialog::ListSelect::setMode( UsageMode mode )
>          _or->hide();
>          _showSelectedOnly->show();
>      }
> +    for ( QTreeWidgetItemIterator itemIt( _treeWidget ); *itemIt; ++itemIt )
> +        configureItem( dynamic_cast<CategoryListView::CheckDropItem*>(*itemIt) );
>  
>      // ensure that the selection count indicator matches the current mode:
>      updateSelectionCount();
> @@ -627,7 +629,6 @@ void AnnotationDialog::ListSelect::configureItem( CategoryListView::CheckDropIte
>  {
>      bool isDNDAllowed = Settings::SettingsData::instance()->viewSortType() == Settings::SortAlphaTree;
>      item->setDNDEnabled( isDNDAllowed && ! _category->isSpecialCategory() );
> -    item->setTristate( _mode == InputMultiImageConfigMode );
>  }
>  
>  bool AnnotationDialog::ListSelect::isInputMode() const

I tried to extract this as patch and apply it to my 4.4 copy:

git diff 24be3677d183dd3d21dd120361097cc6b8815f8a
17e43680e78eb090947c6cfc9bf310486dffa12c > ../kpa4.4/inherit.patch


cd ../kpa4.4
> patch -p1 < inherit.patch

But patch complains about a reverted patch and looking at the (4.4)
source also shows the changes present:
> void AnnotationDialog::ListSelect::setMode( UsageMode mode )
> {
>     _mode = mode;
>     _lineEdit->setMode( mode );
>     if ( mode == SearchMode ) {
>         // "0" below is sorting key which ensures that None is always at top.
>         CheckDropItem* item = new CheckDropItem( _treeWidget, DB::ImageDB::NONE(), QString::fromLatin1("0") );
>         configureItem( item );
>         _and->show();
>         _or->show();
>         _or->setChecked( true );
>         _showSelectedOnly->hide();
>     } else {
>         _and->hide();
>         _or->hide();
>         _showSelectedOnly->show();
>     }
>     for ( QTreeWidgetItemIterator itemIt( _treeWidget ); *itemIt; ++itemIt )
>         configureItem( dynamic_cast<CategoryListView::CheckDropItem*>(*itemIt) );
> 
>     // ensure that the selection count indicator matches the current mode:
>     updateSelectionCount();
> }

Huh ? The change is after the release date. I tried to get more context
from 'git diff' using 'git diff -C 10 ...' - but git complained.

Second try - older patch:
> commit 585656c14b60e280fad05bc9dde19771df807c5b
> Author: Miika Turkia <miika.turkia at gmail.com>
> Date:   Tue Feb 19 19:46:58 2013 +0530
> 
>     Removing unneeded loop
>     
>     Thanks to Robert Krawitz for the fix
> 
> commit d8e6f944ca7bf908794cc6404efeb1fc96e88aa6
> Author: Jesper K. Pedersen <blackie at kde.org>
> Date:   Mon Feb 18 22:46:42 2013 +0100
> 
>     the membergroup section is obvious optional... so support that it is not there

Similar behaviour.
I even tried to accept the reversed patch as reversed - but that gave a
bunch of failed hunks. Before trying to solve these, I would like to
rule out a bad bug on my part...

Could you advise please ?

Thanks

Martin