Repository: rwjblue/ember-cli-template-lint
Pull request #75 (2016-04-11 05:39:10)

neseted interactive rule
Disallow nested interactive elements (`<button>` and `<a>`).
Because of `HTML5` standard technically they cannot be rendered nested, they became sibling. In HTMLBars it possible and I guess it's related to the fact it's JavaScript rendering based. Anyway due of babbling, I don't think you benefit much of that.

[mode info](https://html.spec.whatwg.org/multipage/semantics.html#the-a-element)

**EDIT:** I've copy the todos
- [x] `isInteractiveElement`
- [x] fix the tests
- [x] add info to the README
- [x] enable in default configuration file (generated by blueprint)
- [x] config support per tags (may be default to only `<a>` and `<button>`?)
- [x] cleanups + rebase

rwjblue: This seems good. I left a few inline comments for small tweaks, but we also need to:

- [ ] Fix the tests
- [ ] Add info to the README
- [ ] Enable in default configuration file (generated by blueprint)
oriSomething: to `rebase` everything after the fixes?
rwjblue: Ya, sounds good. 
oriSomething: i've copy the todo, to the top of the thread. i might finish all the other things by tomorrow
oriSomething: @rwjblue please tell me if need to fix anything before rebasing
rwjblue: I made a few inline comments for `isInteractiveElement`, the goal is to have `isInteractiveElement` be an array of functions that we call.

I think we should also allow more configuration options.  Maybe we can start by allowing configuration of the interactive tag names?
oriSomething: what kind of config did you think about?
about making it separate functions. i think i would make them outside the `prototype` of `LogNestedInteractive`, because it feels like i'm polluting them, do you think otherwise?
rwjblue: > about making it separate functions. i think i would make them outside the prototype of LogNestedInteractive, because it feels like i'm polluting them, do you think otherwise?

Personally, I would probably put them on `LogNestedInteractive.prototype` if I were writing it, but I am also perfectly happy to have them defined outside the prototype.

> what kind of config did you think about?

Something like:

```
{
  'nested-interactive': {
    interactiveTagNames: ['button', 'a']
  }
}
```

When `true` we use our default setup, but if `interactiveTagNames` are provided we can use that list instead of our builtin one...
oriSomething: i've updated the todos.
moreover i've separate to smaller functions as you suggested.
oriSomething: i've added the ability to config. note, it's not just about tags. tell me if there is any other thing to fix before rebasing
rwjblue: Looks great, ready for a rebase and squashing!

Patch:
diff --git a/README.md b/README.md
index e861723..5e4742d 100644
--- a/README.md
+++ b/README.md
@@ -146,6 +146,42 @@ This rule forbids the following:
 {{{foo}}}
 ```
 
+
+#### nested-interactive
+
+Usage of nested `interactive content` can lead to UX problems, accessibility
+problems, bugs and in some cases to DOM errors. You should not put interactive
+content elements nested inside other interactive content elements. Instead using
+nested interactive content elements you should separate them and put them one
+after the other.
+
+This rule forbids the following:
+
+```hbs
+<button type="button">
+  Click here and <a href="/">go to home here</a>
+</button>
+```
+
+The following values are valid configuration:
+
+  * boolean -- `true` indicates all whitelist test will run, `false` indicates that the rule is disabled.
+  * array -- an array of whitelisted tests as the following
+
+Whitelist of option in the configuration (are tags name or element attributes):
+
+  * `button`
+  * `details`
+  * `embed`
+  * `iframe`
+  * `img`
+  * `input`
+  * `object`
+  * `select`
+  * `textarea`
+  * `tabindex`
+
+
 ## Contributing
 
 A few ideas for where to take this in the future:
diff --git a/blueprints/ember-cli-template-lint/files/.template-lintrc.js b/blueprints/ember-cli-template-lint/files/.template-lintrc.js
index f3c7002..e86e060 100644
--- a/blueprints/ember-cli-template-lint/files/.template-lintrc.js
+++ b/blueprints/ember-cli-template-lint/files/.template-lintrc.js
@@ -5,5 +5,6 @@ module.exports = {
   'bare-strings': ['(', ')', ',', '.', '&', '+', '-', '=', '*', '/', '#', '%', '!', '?', ':', '[', ']', '{', '}'],
   'block-indentation': 2,
   'html-comments': true,
+  'nested-interactive': true,
   'triple-curlies': true
 };
diff --git a/ext/plugins/index.js b/ext/plugins/index.js
index 09f084f..f30be36 100644
--- a/ext/plugins/index.js
+++ b/ext/plugins/index.js
@@ -4,5 +4,6 @@ module.exports = {
   'bare-strings': require('./lint-bare-strings'),
   'block-indentation': require('./lint-block-indentation'),
   'html-comments': require('./lint-html-comments'),
-  'triple-curlies': require('./lint-triple-curlies')
+  'triple-curlies': require('./lint-triple-curlies'),
+  'nested-interactive': require('./lint-nested-interactive')
 };
diff --git a/ext/plugins/lint-nested-interactive.js b/ext/plugins/lint-nested-interactive.js
new file mode 100644
index 0000000..a0c3483
--- /dev/null
+++ b/ext/plugins/lint-nested-interactive.js
@@ -0,0 +1,228 @@
+'use strict';
+
+/*
+ Disallows nested of interactive elements
+
+ ```
+ {{! good }}
+ <button>Click here</button> <a href="/">and a link</a>
+
+ {{! bad}}
+ <button>Click here <a href="/">and a link</a></button>
+ ```
+
+ The following values are valid configuration:
+
+   * boolean -- `true` for enabled / `false` for disabled
+ */
+
+var calculateLocationDisplay = require('../helpers/calculate-location-display');
+var buildPlugin = require('./base');
+
+module.exports = function(addonContext) {
+  var LogNestedInteractive = buildPlugin(addonContext, 'nested-interactive');
+
+  function isElementNode(node) {
+    return node.type === 'ElementNode';
+  }
+
+  LogNestedInteractive.prototype.parseConfig = function(config) {
+    var configType = typeof config;
+
+    var errorMessage = 'The nested-interactive rule accepts one of the following values.\n ' +
+          '  * boolean - `true` to enable / `false` to disable\n' +
+          '  * array -- an array of strings to whitelist\n' +
+          '\nYou specified `' + JSON.stringify(config) + '`';
+
+    switch (configType) {
+    case 'boolean':
+      return config;
+    case 'object':
+      if (Array.isArray(config)) {
+        return config;
+      } else {
+        throw new Error(errorMessage);
+      }
+    case 'undefined':
+      return false;
+    default:
+      throw new Error(errorMessage);
+    }
+  };
+
+  LogNestedInteractive.prototype.getConfigWhiteList = function() {
+    if (Array.isArray(this.config)) {
+      return this.config;
+    } else {
+      return [
+        'a',
+        'button',
+        'details',
+        'embed',
+        'iframe',
+        'img',
+        'input',
+        'object',
+        'select',
+        'tabindex',
+        'textarea'
+      ];
+    }
+  };
+
+  LogNestedInteractive.prototype.detect = function(node) {
+    return node.type === 'Program';
+  };
+
+  LogNestedInteractive.prototype.process = function(node) {
+    var whitelistTests = this.getConfigWhiteList();
+
+    node.body
+      .filter(isElementNode)
+      .forEach(function(node) {
+        this.findNestedInteractiveElements(node, null, whitelistTests);
+      }, this);
+  };
+
+  LogNestedInteractive.prototype.hasNodeHaveAttribute = function(node, attributeName) {
+    return node.attributes.some(function(attribute) {
+      return attribute.name === attributeName;
+    });
+  };
+
+  LogNestedInteractive.prototype.isNodeLink = function(node, whitelistTests) {
+    return (
+      whitelistTests.indexOf('a') !== -1 &&
+      node.tag === 'a' && this.hasNodeHaveAttribute(node, 'href')
+    );
+  };
+
+  LogNestedInteractive.prototype.isNodeNotHiddenInput = function(node, whitelistTests) {
+    if (whitelistTests.indexOf('input') === -1) {
+      return false;
+    }
+
+    if (node.tag === 'input') {
+      var isInputTypeHidden = node.attributes.some(function(attribute) {
+        return (
+          attribute.name === 'type' &&
+          attribute.value &&
+          attribute.value.chars === 'hidden'
+        );
+      });
+
+      // NOTE: `!`
+      if (!isInputTypeHidden) {
+        return true;
+      }
+    }
+
+    return false;
+  };
+
+  LogNestedInteractive.prototype.hasNodeHaveTabIndex = function(node, whitelistTests) {
+    return (
+      whitelistTests.indexOf('tabindex') !== -1 &&
+      this.hasNodeHaveAttribute(node, 'tabindex')
+    );
+  };
+
+  LogNestedInteractive.prototype.isNodeUseMapInteractive = function(node, whitelistTests) {
+    return (
+      whitelistTests.indexOf(node.tag) !== -1 &&
+      (node.tag === 'img' || node.tag === 'object') &&
+      this.hasNodeHaveAttribute(node, 'usemap')
+    );
+  };
+
+  LogNestedInteractive.prototype.isNodeInteractiveTag = function(node, whitelistTests) {
+    var interactiveTags = [
+      'button',
+      'details',
+      'embed',
+      'iframe',
+      'select',
+      'textarea'
+    ];
+
+    return interactiveTags.some(function(tagName) {
+      return whitelistTests.indexOf(tagName) !== -1 && tagName === node.tag;
+    });
+  };
+
+  /**
+   * NOTE: `<label>` was omitted due to the ability nesting a label with an input tag.
+   * NOTE: `<audio>` and `<video>` also omitted because use legacy browser support
+   * there is a need to use it nested with `<object>` and `<a>`
+   */
+  LogNestedInteractive.prototype.isInteractiveElement = function(node, whitelistTests) {
+    if (!node) {
+      return false;
+    }
+
+    if (this.isNodeInteractiveTag(node, whitelistTests)) {
+      return true;
+    }
+
+    if (this.isNodeLink(node, whitelistTests)) {
+      return true;
+    }
+
+    if (this.isNodeNotHiddenInput(node, whitelistTests)) {
+      return true;
+    }
+
+    if (this.hasNodeHaveTabIndex(node, whitelistTests)) {
+      return true;
+    }
+
+    if (this.isNodeUseMapInteractive(node, whitelistTests)) {
+      return true;
+    }
+
+    return false;
+  };
+
+
+  LogNestedInteractive.prototype.getLogMessage = function(node, parentNode) {
+    var isParentHasTabIndexAttribute = this.hasNodeHaveAttribute(parentNode, 'tabindex');
+    var isParentHasUseMapAttribute = this.hasNodeHaveAttribute(parentNode, 'usemap');
+    var parentNodeError = '<' + parentNode.tag + '>';
+
+    if (isParentHasTabIndexAttribute) {
+      parentNodeError = 'an element with attribute `tabindex`';
+    } else if (isParentHasUseMapAttribute) {
+      parentNodeError = 'an element with attribute `usemap`';
+    }
+
+    var isChildHasTabIndexAttribute = this.hasNodeHaveAttribute(node, 'tabindex');
+    var isChildHasUseMapAttribute = this.hasNodeHaveAttribute(node, 'usemap');
+    var childNodeError = '<' + node.tag + '>';
+
+    if (isChildHasTabIndexAttribute) {
+      childNodeError = 'an element with attribute `tabindex`';
+    } else if (isChildHasUseMapAttribute) {
+      childNodeError = 'an element with attribute `usemap`';
+    }
+
+    return "Don't use " + childNodeError + ' inside ' + parentNodeError + ' ' +
+      calculateLocationDisplay(this.options.moduleName, node.loc.start);
+  };
+
+  LogNestedInteractive.prototype.findNestedInteractiveElements = function(node, parentInteractiveNode, whitelistTests) {
+    if (this.isInteractiveElement(parentInteractiveNode, whitelistTests)) {
+      if (this.isInteractiveElement(node, whitelistTests)) {
+        this.log(this.getLogMessage(node, parentInteractiveNode));
+        return;
+      }
+    }
+
+    node.children
+      .filter(isElementNode)
+      .forEach(function(childNode) {
+        this.findNestedInteractiveElements(childNode, node, whitelistTests);
+      }, this);
+  };
+
+  return LogNestedInteractive;
+};


Test patch:
diff --git a/node-tests/unit/plugins/lint-nested-interactive-test.js b/node-tests/unit/plugins/lint-nested-interactive-test.js
new file mode 100644
index 0000000..674a94f
--- /dev/null
+++ b/node-tests/unit/plugins/lint-nested-interactive-test.js
@@ -0,0 +1,72 @@
+'use strict';
+
+var generateRuleTests = require('../../helpers/rule-test-harness');
+
+generateRuleTests({
+  name: 'nested-interactive',
+
+  config: true,
+
+  good: [
+    '<button>button</button>',
+    '<button>button <strong>!!!</strong></button>',
+    '<a><button>button</button></a>',
+    '<a href="/">link</a>',
+    '<a href="/">link <strong>!!!</strong></a>',
+    '<button><input type="hidden"></button>',
+    {
+      config: ['button', 'details', 'embed', 'iframe', 'img', 'input', 'object', 'select', 'tabindex', 'textarea'],
+      template: '<a href="/">button<a href="/">!</a></a>'
+    }, {
+      config: ['a', 'details', 'embed', 'iframe', 'img', 'input', 'object', 'select', 'tabindex', 'textarea'],
+      template: '<a href="/">button<button>!</button></a>'
+    }, {
+      config: ['a', 'details', 'embed', 'iframe', 'img', 'input', 'object', 'select', 'tabindex', 'textarea'],
+      template: '<button>button<button>!</button></button>'
+    }, {
+      config: ['a', 'button', 'details', 'embed', 'iframe', 'img', 'object', 'select', 'tabindex', 'textarea'],
+      template: '<button><input type="text"></button>'
+    }, {
+      config: ['a', 'button', 'embed', 'iframe', 'img', 'input', 'object', 'select', 'tabindex', 'textarea'],
+      template: '<button><details><summary>Some details</summary><p>!</p></details></button>'
+    }, {
+      config: ['a', 'button', 'details', 'iframe', 'img', 'input', 'object', 'select', 'tabindex', 'textarea'],
+      template: '<button><embed type="video/quicktime" src="movie.mov" width="640" height="480"></button>'
+    }, {
+      config: ['a', 'button', 'details', 'embed', 'img', 'input', 'object', 'select', 'tabindex', 'textarea'],
+      template: '<button><iframe src="/frame.html" width="640" height="480"></iframe></button>'
+    }, {
+      config: ['a', 'button', 'details', 'embed', 'iframe', 'img', 'input', 'object', 'tabindex', 'textarea'],
+      template: '<button><select></select></button>'
+    }, {
+      config: ['a', 'button', 'details', 'embed', 'iframe', 'img', 'input', 'object', 'select', 'tabindex'],
+      template: '<button><textarea></textarea></button>'
+    }, {
+      config: ['a', 'button', 'details', 'embed', 'iframe', 'img', 'input', 'object', 'select', 'textarea'],
+      template: '<div tabindex="1"><button></button></div>'
+    }, {
+      config: ['a', 'button', 'details', 'embed', 'iframe', 'input', 'object', 'select', 'tabindex', 'textarea'],
+      template: '<button><img usemap=""></button>'
+    }, {
+      config: ['a', 'button', 'details', 'embed', 'iframe', 'img', 'input', 'select', 'tabindex', 'textarea'],
+      template: '<object usemap=""><button></button></object>'
+    }
+  ],
+
+  bad: [
+    { config: ['a'], template: '<a href="/">button<a href="/">!</a></a>', message: "Don't use <a> inside <a> (\'layout.hbs\'@ L1:C18)" },
+    { config: ['a', 'button'], template: '<a href="/">button<button>!</button></a>', message: "Don't use <button> inside <a> (\'layout.hbs\'@ L1:C18)" },
+    { config: ['a', 'button'], template: '<button>button<a href="/">!</a></button>', message: "Don't use <a> inside <button> (\'layout.hbs\'@ L1:C14)" },
+    { config: ['button'], template: '<button>button<button>!</button></button>', message: "Don't use <button> inside <button> (\'layout.hbs\'@ L1:C14)" },
+    { config: ['button', 'input'], template: '<button><input type="text"></button>', message: "Don't use <input> inside <button> (\'layout.hbs\'@ L1:C8)" },
+    { config: ['button', 'details'], template: '<button><details><summary>Some details</summary><p>!</p></details></button>', message: "Don't use <details> inside <button> (\'layout.hbs\'@ L1:C8)" },
+    { config: ['button', 'embed'], template: '<button><embed type="video/quicktime" src="movie.mov" width="640" height="480"></button>', message: "Don't use <embed> inside <button> (\'layout.hbs\'@ L1:C8)" },
+    { config: ['button', 'iframe'], template: '<button><iframe src="/frame.html" width="640" height="480"></iframe></button>', message: "Don't use <iframe> inside <button> (\'layout.hbs\'@ L1:C8)" },
+    { config: ['button', 'select'], template: '<button><select></select></button>', message: "Don't use <select> inside <button> (\'layout.hbs\'@ L1:C8)" },
+    { config: ['button', 'textarea'], template: '<button><textarea></textarea></button>', message: "Don't use <textarea> inside <button> (\'layout.hbs\'@ L1:C8)" },
+    { config: ['button', 'tabindex'], template: '<div tabindex="1"><button></button></div>', message: "Don't use <button> inside an element with attribute `tabindex` (\'layout.hbs\'@ L1:C18)" },
+    { config: ['button', 'tabindex'], template: '<button><div tabindex="1"></div></button>', message: "Don't use an element with attribute `tabindex` inside <button> (\'layout.hbs\'@ L1:C8)" },
+    { config: ['button', 'img'], template: '<button><img usemap=""></button>', message: "Don't use an element with attribute `usemap` inside <button> (\'layout.hbs\'@ L1:C8)" },
+    { config: ['button', 'object'], template: '<object usemap=""><button></button></object>', message: "Don't use <button> inside an element with attribute `usemap` (\'layout.hbs\'@ L1:C18)" }
+  ]
+});