isValidColor returns true for non-color strings such as "abc".

[DanJFletcher]

When using the isValidColor method in the HTML validation code, values such as "rg(255,0,0)" will return true, causing a pass in the grader.

My temporary solution in the coding challenge, Colorful Creature:

// CSS Color Names
// Compiled by @bobspace.
//
// A javascript array containing all of the color names listed in the CSS Spec.
// The full list can be found here: http://www.w3schools.com/cssref/css_colornames.asp
// Use it as you please, 'cuz you can't, like, own a color, man.

var CSS_COLOR_NAMES = ["AliceBlue","AntiqueWhite","Aqua","Aquamarine","Azure","Beige","Bisque","Black","BlanchedAlmond","Blue","BlueViolet","Brown","BurlyWood","CadetBlue","Chartreuse","Chocolate","Coral","CornflowerBlue","Cornsilk","Crimson","Cyan","DarkBlue","DarkCyan","DarkGoldenRod","DarkGray","DarkGrey","DarkGreen","DarkKhaki","DarkMagenta","DarkOliveGreen","Darkorange","DarkOrchid","DarkRed","DarkSalmon","DarkSeaGreen","DarkSlateBlue","DarkSlateGray","DarkSlateGrey","DarkTurquoise","DarkViolet","DeepPink","DeepSkyBlue","DimGray","DimGrey","DodgerBlue","FireBrick","FloralWhite","ForestGreen","Fuchsia","Gainsboro","GhostWhite","Gold","GoldenRod","Gray","Grey","Green","GreenYellow","HoneyDew","HotPink","IndianRed","Indigo","Ivory","Khaki","Lavender","LavenderBlush","LawnGreen","LemonChiffon","LightBlue","LightCoral","LightCyan","LightGoldenRodYellow","LightGray","LightGrey","LightGreen","LightPink","LightSalmon","LightSeaGreen","LightSkyBlue","LightSlateGray","LightSlateGrey","LightSteelBlue","LightYellow","Lime","LimeGreen","Linen","Magenta","Maroon","MediumAquaMarine","MediumBlue","MediumOrchid","MediumPurple","MediumSeaGreen","MediumSlateBlue","MediumSpringGreen","MediumTurquoise","MediumVioletRed","MidnightBlue","MintCream","MistyRose","Moccasin","NavajoWhite","Navy","OldLace","Olive","OliveDrab","Orange","OrangeRed","Orchid","PaleGoldenRod","PaleGreen","PaleTurquoise","PaleVioletRed","PapayaWhip","PeachPuff","Peru","Pink","Plum","PowderBlue","Purple","Red","RosyBrown","RoyalBlue","SaddleBrown","Salmon","SandyBrown","SeaGreen","SeaShell","Sienna","Silver","SkyBlue","SlateBlue","SlateGray","SlateGrey","Snow","SpringGreen","SteelBlue","Tan","Teal","Thistle","Tomato","Turquoise","Violet","Wheat","White","WhiteSmoke","Yellow","YellowGreen"];

// returns true if color name is found
// in CSS_COLOR_NAMES.
var isColorName = function(color) {
    var names = [];

    // convert all the names to lower case
    for (name in CSS_COLOR_NAMES)
        names.push(CSS_COLOR_NAMES[name].toLowerCase());

    return names.indexOf(color.toLowerCase()) !== -1;
}

// modified version of isValidColor from live-editor/build/js/live-editor.output_webpage.js
var isColor = constraintPartial(function(color) {
    var isValidNum = function isValidNum(val) {
        var num = parseInt(val, 10);
        return num >= 0 && num <= 255;
    };

    var isRGB = /rgb\((\s*\d+,){2}(\s*\d+\s*)\)/.test(color) || /rgba\((\s*\d+,){3}(\s*\d+\s*)\)/.test(color);
    if (isRGB) {
        var vals = color.split("(")[1].split(",");
        return isValidNum(vals[0]) && isValidNum(vals[1]) && isValidNum(vals[2]);
    }

   // [old]
   //  If they're trying to use a color name, it should be at least
   //  three letters long and not equal to rgb
   //  return /[a-zA-Z]+/.test(color) && color.length >= 3 && color.indexOf("rgb") === -1;

    // [new]
    // are they using a color name?
    return isColorName(color);
});

And then it can be used like this:

cssMatches(<pattern>, isColor(<color>)

Maybe the isValidColor method wasn't intended to be used from the validation code, but it's useful when it works properly :) Is this something worth adding, or is there another way to validate colors that's already implemented?

Thanks in advance for any thoughts.

[pamelafox]

Here's isValidColor in the repo, for context: https://github.com/Khan/live-editor/blob/master/js/output/webpage/webpage-tester.js To avoid having to write out every name in javascript, I did make it pass for a lot of things that aren't valid colors.

We could just bring the array of colors in like your code does or we could have it fail for the known bad cases like rg() - strings that that look like attempts at rgb().

It might also be okay to just have it implemented fully in that challenge. The early challenges are where the majority of newbie color errors will happen, most likely.

Thanks for filing!

[DanJFletcher]

Ok, so would you say it makes sense to leave it as is, until this becomes a higher priority issue then?