WMS: Better logic to pick the legend URL

QGIS had two problems:
1) It was using the specified legend URL only if its mime type was matching
   the layer's mime type. There is no reason for that.
2) When QGIS was using the default layer (empty string), it was not even
   trying to find out in what style to pick the legend URL.

src/providers/wms/qgswmscapabilities.cpp

@@ -907,7 +907,7 @@ void QgsWmsCapabilities::parseLayer( QDomElement const & e, QgsWmsLayerProperty&
 
         for ( int i = 0; i < layerProperty.style.size(); ++i )
         {
-          if ( layerProperty.style[i].name == styleProperty.name )
+          if ( layerProperty.style.at( i ).name == styleProperty.name )
           {
             // override inherited parent's style if it has the same name
             // according to the WMS spec, it should not happen, but Mapserver

src/providers/wms/qgswmsprovider.cpp

@@ -215,6 +215,36 @@ QString QgsWmsProvider::getTileUrl() const
   }
 }
 
+static bool isValidLegend( const QgsWmsLegendUrlProperty &l )
+{
+  return l.format.startsWith( "image/" );
+}
+
+/**
+ * Picks a usable legend URL for a given style.
+ */
+static QString pickLegend( const QgsWmsStyleProperty &s )
+{
+  QString url;
+  for ( int k = 0; k < s.legendUrl.size() && url.isEmpty(); k++ )
+  {
+    const QgsWmsLegendUrlProperty &l = s.legendUrl[k];
+    if ( isValidLegend( l ) )
+    {
+      url = l.onlineResource.xlinkHref;
+    }
+  }
+  return url;
+}
+
+static const QgsWmsStyleProperty *searchStyle( const QVector<QgsWmsStyleProperty>& styles, const QString& name )
+{
+  Q_FOREACH ( const QgsWmsStyleProperty &s, styles )
+    if ( s.name == name )
+      return &s;
+  return nullptr;
+}
+
 QString QgsWmsProvider::getLegendGraphicUrl() const
 {
   QString url;
@@ -223,25 +253,31 @@ QString QgsWmsProvider::getLegendGraphicUrl() const
   {
     const QgsWmsLayerProperty &l = mCaps.mLayersSupported[i];
 
-    if ( l.name != mSettings.mActiveSubLayers[0] )
-      continue;
-
-    for ( int j = 0; j < l.style.size() && url.isEmpty(); j++ )
+    if ( l.name == mSettings.mActiveSubLayers[0] )
     {
-      const QgsWmsStyleProperty &s = l.style[j];
-
-      if ( s.name != mSettings.mActiveSubStyles[0] )
-        continue;
-
-      for ( int k = 0; k < s.legendUrl.size() && url.isEmpty(); k++ )
+      if ( !mSettings.mActiveSubStyles[0].isEmpty() && mSettings.mActiveSubStyles[0] != "default" )
       {
-        const QgsWmsLegendUrlProperty &l = s.legendUrl[k];
-
-        if ( l.format != mSettings.mImageMimeType )
-          continue;
-
-        url = l.onlineResource.xlinkHref;
+        const QgsWmsStyleProperty *s = searchStyle( l.style, mSettings.mActiveSubStyles[0] );
+        if ( s )
+          url = pickLegend( *s );
       }
+      else
+      {
+        // QGIS wants the default style, but GetCapabilities doesn't give us a
+        // way to know what is the default style. So we look for the onlineResource
+        // only if there is a single style available or if there is a style called "default".
+        if ( l.style.size() == 1 )
+        {
+          url = pickLegend( l.style[0] );
+        }
+        else
+        {
+          const QgsWmsStyleProperty *s = searchStyle( l.style, "default" );
+          if ( s )
+            url = pickLegend( *s );
+        }
+      }
+      break;
     }
   }
 

tests/src/providers/CMakeLists.txt

@@ -89,6 +89,10 @@ ADD_QGIS_TEST(wmscapabilititestest
               testqgswmscapabilities.cpp)
 TARGET_LINK_LIBRARIES(qgis_wmscapabilititestest wmsprovider_a)
 
+ADD_QGIS_TEST(wmsprovidertest
+              testqgswmsprovider.cpp)
+TARGET_LINK_LIBRARIES(qgis_wmsprovidertest wmsprovider_a)
+
 #############################################################
 # WCS public servers test:
 # No need to test on all platforms

tests/src/providers/testqgswmsprovider.cpp

@@ -0,0 +1,69 @@
+#include <QFile>
+#include <QObject>
+#include <QtTest/QtTest>
+#include <qgswmsprovider.h>
+#include <qgsapplication.h>
+
+/** \ingroup UnitTests
+ * This is a unit test for the WMS provider.
+ */
+class TestQgsWmsProvider: public QObject
+{
+    Q_OBJECT
+  private slots:
+
+    void initTestCase()
+    {
+      // init QGIS's paths - true means that all path will be inited from prefix
+      QgsApplication::init();
+      QgsApplication::initQgis();
+
+      QFile file( QString( TEST_DATA_DIR ) + "/provider/GetCapabilities.xml" );
+      QVERIFY( file.open( QIODevice::ReadOnly | QIODevice::Text ) );
+      const QByteArray content = file.readAll();
+      QVERIFY( content.size() > 0 );
+      const QgsWmsParserSettings config;
+
+      mCapabilities = new QgsWmsCapabilities();
+      QVERIFY( mCapabilities->parseResponse( content, config ) );
+    }
+
+    //runs after all tests
+    void cleanupTestCase()
+    {
+      delete mCapabilities;
+      QgsApplication::exitQgis();
+    }
+
+    void legendGraphicsWithStyle()
+    {
+      QgsWmsProvider provider( "http://localhost:8380/mapserv?xxx&layers=agri_zones&styles=fb_style&format=image/jpg", mCapabilities );
+      QCOMPARE( provider.getLegendGraphicUrl(), QString( "http://www.example.com/fb.png?" ) );
+    }
+
+    void legendGraphicsWithSecondStyle()
+    {
+      QgsWmsProvider provider( "http://localhost:8380/mapserv?xxx&layers=agri_zones&styles=yt_style&format=image/jpg", mCapabilities );
+      QCOMPARE( provider.getLegendGraphicUrl(), QString( "http://www.example.com/yt.png?" ) );
+    }
+
+    void legendGraphicsWithoutStyleWithDefault()
+    {
+      QgsWmsProvider provider( "http://localhost:8380/mapserv?xxx&layers=buildings&styles=&format=image/jpg", mCapabilities );
+      //only one style, can guess default => use it
+      QCOMPARE( provider.getLegendGraphicUrl(), QString( "http://www.example.com/buildings.png?" ) );
+    }
+
+    void legendGraphicsWithoutStyleWithoutDefault()
+    {
+      QgsWmsProvider provider( "http://localhost:8380/mapserv?xxx&layers=agri_zones&styles=&format=image/jpg", mCapabilities );
+      //two style, cannot guess default => use the WMS GetLegendGraphics
+      QCOMPARE( provider.getLegendGraphicUrl(), QString( "http://localhost:8380/mapserv?" ) );
+    }
+
+  private:
+    QgsWmsCapabilities* mCapabilities;
+};
+
+QTEST_MAIN( TestQgsWmsProvider )
+#include "testqgswmsprovider.moc"