vjs.obj.merge and vjs.createEl don't handle HTML attributes (which are case-insensitive) correctly

[OwenEdwards]

HTML attributes are case-insensitive. JavaScript and vjs.obj.merge are case sensitive, so a merged object may contain two variables which are different only because of their case. vjs.createEl simply iterates through the properties object, applying them to the new element in the order it finds them; the el.setAttribute method it uses is case insensitive, but order matters.

So if this code were used:

vjs.MenuItem.prototype.createEl = function(type, props){
  return vjs.Button.prototype.createEl.call(this, 'li', vjs.obj.merge({
    className: 'vjs-menu-item',
    role: 'menuitem',
    tabindex: -1,
    innerHTML: this.localize(this.options_['label'])
  }, props));
};

....

vjs.Button.prototype.createEl = function(type, props){
  var el;

  // Add standard Aria and Tabindex info
  props = vjs.obj.merge({
    className: this.buildCSSClass(),
    'role': 'button',
    'aria-live': 'polite', // let the screen reader user know that the text of the button may change
    tabIndex: 0
  }, props);

  el = vjs.Component.prototype.createEl.call(this, type, props);

the button element's tabIndex would not be set correctly.

(Note that this problem doesn't seem to have surfaced in the existing code, but it has the potential to; I discovered it while doing some development/enhancement. The attribute tabIndex is already used in both lower-case and camel-case in the existing code, so the potential to cause a problem is there).

[heff]

Good catch. The tabindex in MenuItem is all together wrong. The 'I' needs to be capitalized, at least for the element API to work.

This process is a little complicated too because we've combined properties and attributes into the same process, with some light translation between them, but they're really two different concepts that should be handled separately. We discussed this briefly in a previous issue. The best idea from that was to add a second arg to createEl for handling attributes.

vjs.createEl('div', properties, attributes);

So we need to fix tabIndex first and then improve createEl. Confirming this issue. Anyone feel free to pick it up. We should try to get the createEl change into 5.0.

[heff]

I fixed the tabIndex note in #2204. I'm going to shift the createEl change to 5.1 so we can get 5.0 out, but we should make sure createEl doesn't get exported without this.

[heff]

@OwenEdwards FYI I submitted #2589 to fix the properties/attributes issue.