Merge branch 'master' of https://github.com/GaloisInc/FiveUI

10 files changed, 263 insertions, 66 deletions

diff --git a/doc/build.mk b/doc/build.mk
index f49f83c..a6f340f 100644
--- a/doc/build.mk
+++ b/doc/build.mk
@@ -57,9 +57,9 @@ $(eval $(call stage-doc-dir,$(path)/images))
 
 doc: $(jsdoc-dir)/index.html
 
-$(jsdoc-dir)/index.html:                                      \
-    $(topdir)/src/js/fiveui/injected/prelude.js        \
-    $(topdir)/src/js/fiveui/injected/jquery-plugins.js \
+$(jsdoc-dir)/index.html:                     \
+    src/js/fiveui/injected/prelude.js        \
+    src/js/fiveui/injected/jquery-plugins.js \
   | $(build-dir)
 	$(call label,JSDOC)$(topdir)/tools/bin/jsdoc $^ $(redir)
 
diff --git a/guidelines/accessibility/conformance-A.json b/guidelines/WCAG-1.0/conformance-A.json
index 5aa5bf3..551026c 100644
--- a/guidelines/accessibility/conformance-A.json
+++ b/guidelines/WCAG-1.0/conformance-A.json
@@ -1,7 +1,9 @@
 { "name":        "W3C Accessibility Guidelines (Conformance A)"
-, "description": "See: www.w3.org/TR/WCAG10/"
+, "description": "See: http://www.w3.org/TR/WCAG10/"
 , "license":     "BSD3"
 , "rules":
   [ "guideline-1.js"
+  , "guideline-2.js"
+  , "guideline-3.js"
   ]
 }
diff --git a/guidelines/accessibility/guideline-1.js b/guidelines/WCAG-1.0/guideline-1.js
index fdb2c42..61e1384 100644
--- a/guidelines/accessibility/guideline-1.js
+++ b/guidelines/WCAG-1.0/guideline-1.js
@@ -42,10 +42,12 @@ exports.rule        = function(report) {
 
   // TODO: what's the best way to classify content that's `complex`?
 
-  // All `area` elements of an image map should have alt attributes.  This isn't
-  // quite a faithful implementation, as it doesn't take into account the case
-  // where an `a` tag is wrapped around the `area` tag.
-  $5('map').find('area').each(hasAlt);
+  // All `area` elements of an image map should have alt attributes. It's also a
+  // bit overzealous, as it looks at all maps, not just maps that are referenced
+  // from images.
+  $5('map').find('area')
+    .add($5('map').find('a'))
+      .each(hasAlt);
 
   // TODO: figure out a good way to handle frames.
   // TODO: figure out a good way to handle scripts.
@@ -53,19 +55,41 @@ exports.rule        = function(report) {
 
   /* Checkpoint 1.2 [Priority 1] **********************************************/
 
-  // TODO
+  // `ismap` is a boolean attribute.  If it's present on an image, require that
+  // there's also a corresponding `usemap` attribute that can be used in lieu of
+  // having the server information present.  This is a bit of an under
+  // approximation, as if you can provide a client-side map that doesn't cover
+  // everything that the server does.  It's more of a sanity check, that this
+  // has been thought of.
+  $5('img').filter('[ismap]').each(function(ix) {
+    if(_.isEmpty($(this).attr('usemap'))) {
+      report.error('No usemap attribute to supplement a use of ismap', this);
+    }
+  });
 
 
   /* Checkpoint 1.3 [Priority 1] **********************************************/
 
-  // TODO
+  // TODO: Not really sure if this is something that we can check; the guideline
+  // seems to be more of a subjective check.
 
 
   /* Checkpoint 1.4 [Priority 1] **********************************************/
 
-  // TODO
+  // TODO: Again, not sure if this is something we can check here.
 
 
   /* Checkpoint 1.5 [Priority 3] **********************************************/
 
+  // Make sure that every link in an image map has a corresponding text link
+  // somewhere else in the document.
+  var hrefs = $5('a').map(function() { return $(this).attr('href'); });
+  $5('map').find('area').each(function() {
+    var href = $(this).attr('href');
+    if(!_.contains(href, hrefs)) {
+      report.error('Image map contains a link not present in a text link',
+          this);
+    }
+  });
+
 };
diff --git a/guidelines/WCAG-1.0/guideline-2.js b/guidelines/WCAG-1.0/guideline-2.js
new file mode 100644
index 0000000..7dcf28d
--- /dev/null
+++ b/guidelines/WCAG-1.0/guideline-2.js
@@ -0,0 +1,43 @@
+exports.name        = "colorDifference";
+exports.description = "Elements should provide sufficient color difference";
+exports.rule        = function(report) {
+
+  /* Checkpoint 2.1 ***********************************************************/
+
+  // TODO: not sure about the best way to test that information isn't hidden
+  // when colors go away.
+
+
+  /* Checkpoint 2.2 ***********************************************************/
+
+  var fc = fiveui.color;
+  var MIN_COLOR_DIFF = 500; // http://www.w3.org/TR/2000/WD-AERT-20000426#color
+
+  /**
+   * Return the absolute "color difference" between two * given RGB color
+   * objects.
+   * Input is two RGB color objects.
+   */
+  var colorDiff = function (c1, c2) {
+    return Math.abs(c1.r - c2.r) +
+           Math.abs(c1.g - c2.g) +
+           Math.abs(c1.b - c2.b);
+  };
+
+  fiveui.query('*')
+    .filter(function () {  // filter for lowest level elts having non-empty text
+      var $this = $(this);
+      return $this.children().length == 0 && $.trim($this.text()).length > 0;
+    })
+    .each(function (i) {
+      // TODO take into account fg alpha values
+      var fg = fc.colorToRGB($(this).css('color'));
+      var bg = fc.findBGColor($(this));
+      if (fg && bg) {
+        var diff = colorDiff(fg, bg);
+        if (diff < MIN_COLOR_DIFF) {
+          report.error('Element has poor color difference: ' + diff, this);
+        }
+      }
+  });
+};
diff --git a/guidelines/WCAG-1.0/guideline-3.js b/guidelines/WCAG-1.0/guideline-3.js
new file mode 100644
index 0000000..1be4354
--- /dev/null
+++ b/guidelines/WCAG-1.0/guideline-3.js
@@ -0,0 +1,66 @@
+
+exports.name        = 'W3C Guideline 3';
+exports.description = '';
+exports.rule        = function(report) {
+
+  /* Checkpoint 3.1 [Priority 2] **********************************************/
+
+  // TODO: this seems pretty subjective, as you have to be able to understand
+  // the intent of the content.  The math example is tough, as you'd have to be
+  // able to pick out a situation where text wasn't marked up, but was also
+  // mathematical notation.
+
+
+  /* Checkpoint 3.2 [Priority 2] **********************************************/
+
+  // require that the document contains a dtd.
+  // TODO: how should we apply this check to iframes and such?
+  if(!document.doctype) {
+    report.error('No doctype given for the document', null);
+  }
+
+
+  /* Checkpoint 3.3 [Priority 2] **********************************************/
+
+  // use style sheets instead of HTML attributes to specify formatting
+  // information.
+  $5('b').each(function() {
+    report.error('The b tag shouldn\'t be used, use strong instead', this);
+  });
+
+  $5('i').each(function() {
+    report.error('The i tag shouldn\'t be used, use em', this);
+  });
+
+  $5('[font]').each(function() {
+    report.error('Use css instead of the font attribute for formatting', this);
+  });
+
+  // TODO: there are other cases to handle here, not sure about the best path
+  // forward.
+
+
+  /* Checkpoint 3.4 [Priority 2] **********************************************/
+
+  // TODO: not sure what the best way to select everything that's not
+  // automatically positioned.  Additionally, many fancy user interfaces will
+  // use pixels when positioning content, which isn't necessarily wrong.
+
+
+  /* Checkpoint 3.5 [Priority 2] **********************************************/
+
+  // TODO: what's the best way to select siblings that match a given pattern in
+  // jquery?  Essentially, we just want to match situations where h1 is followed
+  // by something that's both a header, and not h2 (for example).
+
+
+  /* Checkpoint 3.6 [Priority 2] **********************************************/
+
+
+  /* Checkpoint 3.7 [Priority 2] **********************************************/
+
+  // TODO: is there any way that we can detect quotations that aren't inside of
+  // a blockquote region?
+
+
+};
diff --git a/src/js/fiveui/injected/compute.js b/src/js/fiveui/injected/compute.js
index db6e650..921229e 100644
--- a/src/js/fiveui/injected/compute.js
+++ b/src/js/fiveui/injected/compute.js
@@ -95,13 +95,16 @@
    };
 
    core.hash = function(rule, message, node) {
+
      var prob = {
-       name: rule.name,
-       msg: message,
-       descr: rule.description,
-       url: window.location.href,
+       name:     rule.name,
+       msg:      message,
+       descr:    rule.description,
+       url:      window.location.href,
        severity: 1,
-       xpath: core.getElementXPath(node)
+       xpath:    core.getElementXPath(node),
+       phash:    null,
+       hash:     null,
      };
 
      var nodeParents = function(node) {
@@ -125,7 +128,9 @@
      var str = prob.name + prob.descr + prob.url + prob.severity
              + name + nodeHash(node);
 
-     prob.hash = hex_md5(str); // hex_md5() is from md5.js
+     // hex_md5() is from md5.js
+     prob.hash  = hex_md5(str);
+     prob.phash = hex_md5(str + message);
 
      return prob;
    };
@@ -237,10 +242,10 @@
        error:function(message, node) {
          var prob = core.hash(theRule, message, node);
          var query = $(node);
-         if(!query.hasClass(prob.hash)) {
-           query.addClass(prob.hash);
-           core.reportProblem(prob);
-         }
+
+         // let the backend sort out if this problem has been reported already
+         query.addClass(prob.hash);
+         core.reportProblem(prob);
        }
      };
 
diff --git a/src/js/fiveui/injected/ui.js b/src/js/fiveui/injected/ui.js
index d742720..1eecd93 100644
--- a/src/js/fiveui/injected/ui.js
+++ b/src/js/fiveui/injected/ui.js
@@ -62,27 +62,55 @@
      }, 10);
    };
 
-   core.highlightProblem = function(elt) {
-     core.maskRules(function() {
+   core.highlighted = {};
+
+   core.highlightProblem = function(prob) {
+     var obj = core.highlighted[prob.hash];
+     if(obj) {
+       // increment the number of times this has been highlighted
+       obj.highlighted = obj.highlighted + 1;
+     } else {
+       // add the rule to the list of highlighted elements, and change its style
+       // to look obvious.
+       var elt      = fiveui.query('.' + prob.hash);
        var oldStyle = elt.attr('style');
 
-       elt.attr('style', 'background-color: rgba(255,0,0,0.3); background-image: none;');
-       elt.addClass('uic-problem');
+       core.maskRules(function() {
+         elt.attr('style', 'background-color: rgba(255,0,0,0.3); background-image: none;');
+         elt.addClass('uic-problem');
+       });
 
-       return oldStyle;
-     });
+       // record the element for the future
+       core.highlighted[prob.hash] = {
+         highlighted: 1,
+         oldStyle:    oldStyle,
+       }
+     }
    };
 
-   core.maskProblem = function(elt, oldStyle) {
-     core.maskRules(function() {
-       if (oldStyle == undefined) {
-         elt.removeAttr('style');
-       } else {
-         elt.attr('style', oldStyle);
-       }
+   core.maskProblem = function(prob) {
+     var obj = core.highlighted[prob.hash];
 
-       elt.removeClass('uic-problem');
-     });
+     if(obj) {
+       obj.highlighted = obj.highlighted - 1;
+
+       if(obj.highlighted == 0) {
+         var elt = fiveui.query('.' + prob.hash);
+
+         // remove the fiveui style
+         core.maskRules(function() {
+           if (_.isEmpty(obj.oldStyle)) {
+             elt.removeAttr('style');
+           } else {
+             elt.attr('style', obj.oldStyle);
+           }
+
+           elt.removeClass('uic-problem');
+         });
+
+         delete core.highlighted[prob.hash];
+       }
+     }
    };
 
    core.renderStatsTemplate = _.template(
@@ -170,18 +198,17 @@
 
        prExpand.click(
          function() {
-           var oldStyle;
            var elt = $(this);
            if(elt.is('.prExpand-down')) {
              elt.removeClass('prExpand-down')
                 .addClass('prExpand-right');
              prDetails.hide();
-             core.maskProblem(fiveui.query('.' + prob.hash), oldStyle);
+             core.maskProblem(prob);
            } else {
              elt.addClass('prExpand-down')
                 .removeClass('prExpand-right');
              prDetails.show();
-             oldStyle = core.highlightProblem(fiveui.query('.' + prob.hash));
+             core.highlightProblem(prob);
            }
 
            return false;
diff --git a/src/js/fiveui/js/ffcheck.js b/src/js/fiveui/js/ffcheck.js
deleted file mode 100644
index 2157ed0..0000000
--- a/src/js/fiveui/js/ffcheck.js
+++ /dev/null
@@ -1,3 +0,0 @@
-if (! /Firefox[\/\s](\d+\.\d+)/.test(navigator.userAgent)) {
-  window.onload=fiveui.chrome.options.init;
-}
diff --git a/src/js/fiveui/js/state.js b/src/js/fiveui/js/state.js
index b3487c5..903f047 100644
--- a/src/js/fiveui/js/state.js
+++ b/src/js/fiveui/js/state.js
@@ -41,19 +41,28 @@ fiveui.WinState = function(x, y, width, height, closed) {
 
 /**
  * @constructor
- * @param {string} name The name of the rule that this problem represents.
- * @param {string} descr Short description of the problem.
- * @param {string} url The url that the problem occurred at.
- * @param {number} severity The severity of the problem
+ * @param {string} cfg The config object for the problem.  See
+ *                     fiveui.Problem.sanitize
  */
-fiveui.Problem = function(name, descr, url, severity, hash, xpath, msg) {
-  this.name = name || '';
-  this.descr = descr || '';
-  this.url = url || '';
-  this.severity = severity || 0;
-  this.hash = hash;
-  this.xpath = xpath;
-  this.msg = msg;
+fiveui.Problem = function(cfg) {
+  _.defaults(this, fiveui.Problem.sanitize(cfg));
+};
+
+fiveui.Problem.sanitize = function(obj) {
+
+  var defs = {
+    name:     '',   // the name of the rule that this problem came from
+    descr:    '',   // short description of the problem
+    url:      '',   // url that the problem came from
+    severity: 0,    // severity of the problem
+    phash:    null, // hash for the combination of problem and element in context
+    hash:     null, // hash for the element in context
+    xpath:    '',   // path of the element in the document
+    msg:      '',   // problem message
+  };
+
+  return _.defaults(_.pick(obj, _.keys(defs)), defs);
+
 };
 
 /**
@@ -61,7 +70,7 @@ fiveui.Problem = function(name, descr, url, severity, hash, xpath, msg) {
  * @return {!fiveui.Problem} The problem that the object represents.
  */
 fiveui.Problem.fromJSON = function(obj) {
-  return new fiveui.Problem(obj.name, obj.descr, obj.url, obj.severity, obj.hash, obj.xpath, obj.msg);
+  return new fiveui.Problem(obj);
 };
 
 /**
@@ -73,26 +82,32 @@ fiveui.Problem.fromJSON = function(obj) {
  *                                   the corresponding tab.
  */
 fiveui.TabState = function(tabId, winState, uiPort) {
-  this.tabId = tabId;
-  this.winState = winState;
-  this.uiPort = uiPort;
+  this.tabId        = tabId;
+  this.winState     = winState;
+  this.uiPort       = uiPort;
   this.computePorts = [];
-  this.problems = [];
+  this.problems     = [];
   this.seenProblems = new Set();
-  this.stats = {};
+  this.stats        = {};
 };
 
 _.extend(fiveui.TabState.prototype, {
 
+  /**
+   * Returns true when the combination of element and problem has been seen
+   * before, to avoid repeats.
+   *
+   * NOTE:  we use phash here, as it allows multiple distinct problems to target
+   * the same element.
+   */
   addProblem: function(prob) {
-    if(!this.seenProblems.contains(prob.hash)) {
+    if(this.seenProblems.contains(prob.phash)) {
+      return false;
+    } else {
       this.problems.push(prob);
-      this.seenProblems.add(prob.hash);
+      this.seenProblems.add(prob.phash);
       return true;
     }
-    else {
-      return false;
-    }
   },
 
   addStats: function (stats) {
diff --git a/src/js/fiveui/js/utils.js b/src/js/fiveui/js/utils.js
index 204d961..9f4d2c6 100644
--- a/src/js/fiveui/js/utils.js
+++ b/src/js/fiveui/js/utils.js
@@ -98,6 +98,8 @@ var removeComments = function(data) {
   var state     = 0;
   var toEOL     = 1;
   var toEOC     = 2;
+  var inQUOTE   = 3;
+  var inDQUOTE  = 4;
 
   var sanitized = '';
   var len       = data.length;
@@ -120,8 +122,24 @@ var removeComments = function(data) {
         }
         break;
 
+      case inQUOTE:
+        if(data[e] == '\'') {
+          state = 0;
+        }
+        break;
+
+      case inDQUOTE:
+        if(data[e] == '"') {
+          state = 0;
+        }
+        break;
+
       default:
-        if(data[e] == '/') {
+        if(data[e] == '\'') {
+          state = inQUOTE;
+        } else if(data[e] == '"') {
+          state = inDQUOTE;
+        } else if(data[e] == '/') {
           if(data[e+1] == '/') {
             sanitized = sanitized + data.substring(s,e);
             state     = toEOL;