Bug 243562

Created attachment 48582 [details]
How it should look

Created attachment 48583 [details]
How it looks

Could you please update to Amarok 2.3.1 and try again? You can find more information about it here: http://kubuntu.org/news/amarok-2.3.1

I just reproduced the issue in  amarok 2.3.1.

Thank you for the feedback.

*** Bug 247898 has been marked as a duplicate of this bug. ***

Confirmed by duplicate.

confirmed in amarok master with the test above

The fix is in reviewboard.

One additional problem that was not mentioned: PlaylistDuration was behaving strange.

commit 4d2725cd4f946d0a4ffb0acbe564103ea4b5c535
Author: Mark Kretschmann <kretschmann@kde.org>
Date:   Fri Oct 15 09:58:55 2010 +0200

    Change order of setting filter and value (one resets the other)
    First set the values and then connect the signal
    For the layout set the layout policy.
    
    Patch by Ralf Engels <ralf-engels@gmx.de>.
    
    BUG: 243562

diff --git a/ChangeLog b/ChangeLog
index 68f7c32..2075097 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -26,6 +26,7 @@ VERSION 2.4
       pressing SHIFT while clicking the action will bypass trash.
 
   BUGFIXES:
+    * Dynamic Playlist criteria were not being saved properly. (BR 243562)
     * Collection Browser should show Artist names for Compilation Albums. (BR
       252790)
     * When Amarok starts and "Continue playing when Amarok is started" is selected,
diff --git a/src/browsers/playlistbrowser/DynamicBiasWidgets.cpp b/src/browsers/playlistbrowser/DynamicBiasWidgets.cpp
index ae1eb19..00024b7 100644
--- a/src/browsers/playlistbrowser/DynamicBiasWidgets.cpp
+++ b/src/browsers/playlistbrowser/DynamicBiasWidgets.cpp
@@ -182,21 +182,24 @@ PlaylistBrowserNS::BiasGlobalWidget::BiasGlobalWidget(
     m_weightSelection = new Amarok::Slider( Qt::Horizontal, 100, frame );
     m_weightSelection->setToolTip(
             i18n( "This controls what portion of the playlist should match the criteria" ) );
-    connect( m_weightSelection, SIGNAL(valueChanged(int)),
-            this, SLOT(weightChanged(int)) );
 
     QHBoxLayout* sliderLayout = new QHBoxLayout();
     sliderLayout->addWidget( m_weightSelection );
     sliderLayout->addWidget( m_weightLabel );
 
     m_queryWidget = new MetaQueryWidget( frame );
-    connect( m_queryWidget, SIGNAL(changed(const MetaQueryWidget::Filter&)), this, SLOT(syncBiasToControls()));
-
+    m_queryWidget->setSizePolicy( QSizePolicy( QSizePolicy::MinimumExpanding,
+                                               QSizePolicy::Preferred ) );
     layout->addRow( i18n( "Proportion:" ), sliderLayout );
     layout->addRow( i18n( "Match:" ), m_queryWidget );
 
     syncControlsToBias();
 
+    connect( m_weightSelection, SIGNAL(valueChanged(int)),
+             SLOT(weightChanged(int)) );
+    connect( m_queryWidget, SIGNAL(changed(const MetaQueryWidget::Filter&)),
+             SLOT(syncBiasToControls()));
+
     this->layout()->addWidget( frame );
 }
 
@@ -251,21 +254,22 @@ PlaylistBrowserNS::BiasNormalWidget::BiasNormalWidget( Dynamic::NormalBias* bias
     m_scaleSelection = new Amarok::Slider( Qt::Horizontal, 100, frame );
     m_scaleSelection->setToolTip(
             i18n( "This controls how strictly to match the given value." ) );
-    connect( m_scaleSelection, SIGNAL(valueChanged(int)),
-            SLOT(scaleChanged(int)) );
-
     QHBoxLayout* sliderLayout = new QHBoxLayout();
     sliderLayout->addWidget( m_scaleSelection );
     sliderLayout->addWidget( m_scaleLabel );
 
     m_queryWidget = new MetaQueryWidget( frame, true, true );
-    connect( m_queryWidget, SIGNAL(changed(const MetaQueryWidget&)), this, SLOT(syncBiasToControls()));
-
     layout->addRow( i18n( "Strictness:" ), sliderLayout );
     layout->addRow( i18n( "Match:" ), m_queryWidget );
 
     syncControlsToBias();
 
+    connect( m_scaleSelection, SIGNAL(valueChanged(int)),
+             SLOT(scaleChanged(int)) );
+    connect( m_queryWidget, SIGNAL(changed(const MetaQueryWidget::Filter&)),
+             SLOT(syncBiasToControls()));
+
+
     this->layout()->addWidget( frame );
 }
 
@@ -277,17 +281,19 @@ PlaylistBrowserNS::BiasNormalWidget::syncControlsToBias()
     scaleChanged(scale); // the widget value might not have changed and thus the signal not fired
 
     MetaQueryWidget::Filter filter;
-    filter.numValue = m_nbias->value();
     filter.field    = m_nbias->field();
+    filter.numValue = m_nbias->value();
 
     m_queryWidget->setFilter( filter );
+
+    filter = m_queryWidget->filter();
 }
 
 void
 PlaylistBrowserNS::BiasNormalWidget::syncBiasToControls()
 {
-    m_nbias->setValue( m_queryWidget->filter().numValue );
     m_nbias->setField( m_queryWidget->filter().field );
+    m_nbias->setValue( m_queryWidget->filter().numValue );
     m_nbias->setScale( m_scaleSelection->value() / 100.0 );
     m_nbias->setActive( true );
 
diff --git a/src/widgets/MetaQueryWidget.cpp b/src/widgets/MetaQueryWidget.cpp
index 5f9171a..ae07e6b 100644
--- a/src/widgets/MetaQueryWidget.cpp
+++ b/src/widgets/MetaQueryWidget.cpp
@@ -124,6 +124,7 @@ MetaQueryWidget::MetaQueryWidget( QWidget* parent, bool onlyNumeric, bool noCond
     : QWidget( parent )
     , m_onlyNumeric( onlyNumeric )
     , m_noCondition( noCondition )
+    , m_settingFilter( false )
     , m_andLabel(0)
     , m_compareSelection(0)
     , m_valueSelection1(0)
@@ -140,9 +141,9 @@ MetaQueryWidget::MetaQueryWidget( QWidget* parent, bool onlyNumeric, bool noCond
     m_layoutMain->addLayout(m_layoutValue);
 
     m_layoutValueLabels = new QVBoxLayout();
-    m_layoutValue->addLayout(m_layoutValueLabels);
+    m_layoutValue->addLayout(m_layoutValueLabels, 0);
     m_layoutValueValues = new QVBoxLayout();
-    m_layoutValue->addLayout(m_layoutValueValues);
+    m_layoutValue->addLayout(m_layoutValueValues, 1);
 
     if( m_onlyNumeric ) {
         m_filter.field = Meta::valYear;
@@ -176,6 +177,7 @@ MetaQueryWidget::filter() const
 void
 MetaQueryWidget::setFilter( const MetaQueryWidget::Filter &value )
 {
+    m_settingFilter = true;
     m_filter = value;
 
     // correct filter
@@ -187,13 +189,13 @@ MetaQueryWidget::setFilter( const MetaQueryWidget::Filter &value )
 
     int index = m_fieldSelection->findData( (int)m_filter.field );
     m_fieldSelection->setCurrentIndex( index == -1 ? 0 : index );
-    m_filter = value; // the value can be reset by setFilter
 
     if( !m_noCondition )
         makeCompareSelection();
     makeValueSelection();
     setValueSelection();
 
+    m_settingFilter = false;
     emit changed(m_filter);
 }
 
@@ -239,6 +241,9 @@ MetaQueryWidget::makeFieldSelection()
 void
 MetaQueryWidget::fieldChanged( int i )
 {
+    if( m_settingFilter )
+        return;
+
     qint64 field = qvariant_cast<qint64>( m_fieldSelection->itemData( i ) );
     if( m_filter.field == field )
         return; // nothing to do
@@ -515,7 +520,6 @@ void
 MetaQueryWidget::makeGenericComboSelection( bool editable, Collections::QueryMaker* populateQuery )
 {
     KComboBox* combo = new KComboBox( this );
-    combo->setSizePolicy( QSizePolicy::Ignored, QSizePolicy::Preferred );
     combo->setEditable( editable );
 
     if( populateQuery != 0 )
diff --git a/src/widgets/MetaQueryWidget.h b/src/widgets/MetaQueryWidget.h
index 4d544c7..c720b6d 100644
--- a/src/widgets/MetaQueryWidget.h
+++ b/src/widgets/MetaQueryWidget.h
@@ -169,6 +169,8 @@ class MetaQueryWidget : public QWidget
         bool m_onlyNumeric;
         bool m_noCondition;
 
+        bool m_settingFilter; // if set to true we are just setting the filter
+
         QVBoxLayout* m_layoutMain;
         QHBoxLayout* m_layoutValue;
         QVBoxLayout* m_layoutValueLabels;