Bug 128798

SVN commit 550790 by arwalker:

BUG:128798 Correctly handle colors given by r,g,b format instead of crashing

 M  +40 -6     labelparser.cpp  


--- trunk/extragear/graphics/kst/src/libkstmath/labelparser.cpp #550789:550790
@@ -20,6 +20,7 @@
 #include <assert.h>
 #include <stdlib.h>
 
+#include <qregexp.h>
 #include <qstring.h>
 
 using namespace Label;
@@ -114,19 +115,52 @@
 
 
 inline QColor parseColor(const QString& txt, int *skip) {
+  QColor color;
   const int end = txt.find('}');
   if (skip) {
     *skip = end;
   }
+  
   if (end == -1) {
-    return QColor();
+    return color;
   }
-  if (txt[0].isNumber()) {
-    abort(); // FIXME: implement RR,GG,BB hex
-  } else if (txt[0].isLetter() || txt[0] == '#') {
-    return QColor(txt.left(end)); // Use the X11 rgb database, or #RRGGBB
+  
+  color.setNamedColor(txt.left(end));
+  if (!color.isValid()) {
+    // the color is in the format "r,g,b"
+    QStringList strColors = QStringList::split(QRegExp(","), txt.left(end), TRUE);
+    if (strColors.size() == 3) {
+      QString strColor;
+      bool ok = true;
+      int colors[3];
+      int base = 10;
+      
+      // assume the colors are given as decimal numbers unless we have a hex value in the string
+      if (txt.left(end).find(QRegExp("[A-F]", FALSE)) != -1) {
+        base = 16;
+      }
+
+      for (int i=0; i<3; i++) {
+        strColor = strColors[i];
+        strColor.stripWhiteSpace();
+        if (strColors[i].isEmpty()) {
+          colors[i] = 0;
+        } else {
+          colors[i] = strColors[i].toInt(&ok, base);
+        }
+        
+        if (!ok) {
+          break;
+        }
+      }
+      
+      if (ok) {
+        color.setRgb(colors[0],colors[1],colors[2]);
+      }
+    }
   }
-  return QColor();
+  
+  return color;
 }
 
 

  This patch is quite inefficient in a codepath that is supposed to be as fast 
as possible.  Furthermore, the abort() was left in because I wasn't sure what 
the right behavior should be.  We can parse #RRGGBB already, so I'd rather 
stick with just that for now.  If the abort() is really a problem, we can 
comment it out.

 Oh and remember, any patches to code that has testcases requires (read: no 
longer optional) updated testcases.  Labelparser has nearly 100% coverage so 
any changes there must be accompanied by testcases.