From 4af8af02e5c483467cbd475b1e7f972e2851cbd5 Mon Sep 17 00:00:00 2001 From: David Date: Thu, 24 Jul 2014 16:36:03 -0700 Subject: [PATCH 01/13] add test for video tag attr conservation --- test/unit/player.js | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/unit/player.js b/test/unit/player.js index 7eb6db94e6..d23fb6d03f 100644 --- a/test/unit/player.js +++ b/test/unit/player.js @@ -482,3 +482,32 @@ test('player should handle different error types', function(){ vjs.log.error.restore(); }); +test('should restore all video tags attribute after a tech switch', function(){ + var fixture = document.getElementById('qunit-fixture'); + var html = ''; + fixture.innerHTML += html; + + var tag = document.getElementById('example_1'); + var player = new videojs.Player(tag, {techOrder:['html5']}); + var techOptions = vjs.obj.merge({ 'source': '', 'parentEl': player.el_ }, player.options_['html5']); + vjs.Html5.disposeMediaElement(player.tag); + player.tag = null; + player.tech = new vjs.Html5(player, techOptions); + + equal(player.tech.el_.outerHTML,html.replace('example_1','example_1_html5_api')); +}); + +test('should restore all video tags attribute after a tech switch and keep options', function(){ + var fixture = document.getElementById('qunit-fixture'); + var html = ''; + fixture.innerHTML += html; + + var tag = document.getElementById('example_1'); + var player = new videojs.Player(tag, {techOrder:['html5'], autoplay:false}); + var techOptions = vjs.obj.merge({ 'source': '', 'parentEl': player.el_ }, player.options_['html5']); + vjs.Html5.disposeMediaElement(player.tag); + player.tag = null; + player.tech = new vjs.Html5(player, techOptions); + + equal(player.tech.el_.outerHTML,html.replace('example_1','example_1_html5_api').replace(' autoplay=""','')); +}); From c4aa9faca73df09737e17465eade3ecc4e3c24a9 Mon Sep 17 00:00:00 2001 From: David Date: Thu, 24 Jul 2014 16:36:58 -0700 Subject: [PATCH 02/13] set attributes of video tag and not only values --- src/js/media/html5.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/js/media/html5.js b/src/js/media/html5.js index 19dcc3cf7b..db61fb2b5a 100644 --- a/src/js/media/html5.js +++ b/src/js/media/html5.js @@ -96,6 +96,17 @@ vjs.Html5.prototype.createEl = function(){ for (var i = attrs.length - 1; i >= 0; i--) { var attr = attrs[i]; if (player.options_[attr] !== null) { + if(typeof player.options_[attr] === 'boolean') { + if(player.options_[attr]) { + el.setAttribute(attr,''); + } else { + el.removeAttribute(attr); + } + } else if(typeof player.options_[attr] === 'undefined') { + el.removeAttribute(attr); + } else { + el.setAttribute(attr,player.options_[attr]); + } el[attr] = player.options_[attr]; } } From 2537c98c688d8853b5e592c4ccfc9ad0120e807a Mon Sep 17 00:00:00 2001 From: David Date: Thu, 24 Jul 2014 17:12:33 -0700 Subject: [PATCH 03/13] add unsupported attribute to the video tag - test failing --- test/unit/player.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/player.js b/test/unit/player.js index d23fb6d03f..cc37647296 100644 --- a/test/unit/player.js +++ b/test/unit/player.js @@ -484,7 +484,7 @@ test('player should handle different error types', function(){ test('should restore all video tags attribute after a tech switch', function(){ var fixture = document.getElementById('qunit-fixture'); - var html = ''; + var html = ''; fixture.innerHTML += html; var tag = document.getElementById('example_1'); From 0e78c9e53c7710c693d1758d8f3692d85053cda0 Mon Sep 17 00:00:00 2001 From: David Date: Thu, 24 Jul 2014 18:17:29 -0700 Subject: [PATCH 04/13] helper to set attributes on an element from a map of values --- src/js/lib.js | 19 +++++++++++++++++++ test/unit/lib.js | 16 ++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/js/lib.js b/src/js/lib.js index 52abda9f87..ee87c57888 100644 --- a/src/js/lib.js +++ b/src/js/lib.js @@ -34,6 +34,25 @@ vjs.createEl = function(tagName, properties){ return el; }; +/** + * Apply attributes to an html element. + * @param {Element} el Target element. + * @param {Object=} attributes Element attributes to be applied. + * @private + */ +vjs.setElementAttributes = function(el, attributes){ + var keys = Object.keys(attributes); + for(var i = 0,l = keys.length;i Date: Thu, 24 Jul 2014 18:18:22 -0700 Subject: [PATCH 05/13] dummy compare of html content with a sort of the attributes --- test/unit/test-helpers.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/unit/test-helpers.js b/test/unit/test-helpers.js index ee91ecb4c7..ac84970f94 100644 --- a/test/unit/test-helpers.js +++ b/test/unit/test-helpers.js @@ -17,5 +17,16 @@ var PlayerTest = { playerOptions['techOrder'] = ['mediaFaker']; return player = new videojs.Player(videoTag, playerOptions); + }, + htmlEqualWithSort : function(htmlResult,htmlExpected) { + function htmlTransform(str) { + str = str.replace(/[<|>]/g,' '); + str = str.trim(); + str = str.replace(/\s{2,}/g, ' '); + return str.split(' ').sort().join(' '); + } + htmlResult= htmlResult.split(' ').sort().join(' '); + equal(htmlTransform(htmlResult),htmlTransform(htmlExpected)); + } }; From dae47c68fd606c94c21dbd67bc64c5e0bdba3d09 Mon Sep 17 00:00:00 2001 From: David Date: Thu, 24 Jul 2014 18:18:57 -0700 Subject: [PATCH 06/13] ignore html attributes order for comparition --- test/unit/player.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/player.js b/test/unit/player.js index cc37647296..fac7ced4f3 100644 --- a/test/unit/player.js +++ b/test/unit/player.js @@ -494,7 +494,7 @@ test('should restore all video tags attribute after a tech switch', function(){ player.tag = null; player.tech = new vjs.Html5(player, techOptions); - equal(player.tech.el_.outerHTML,html.replace('example_1','example_1_html5_api')); + PlayerTest.htmlEqualWithSort(player.tech.el_.outerHTML, html.replace('example_1','example_1_html5_api')); }); test('should restore all video tags attribute after a tech switch and keep options', function(){ @@ -509,5 +509,5 @@ test('should restore all video tags attribute after a tech switch and keep optio player.tag = null; player.tech = new vjs.Html5(player, techOptions); - equal(player.tech.el_.outerHTML,html.replace('example_1','example_1_html5_api').replace(' autoplay=""','')); + PlayerTest.htmlEqualWithSort(player.tech.el_.outerHTML, html.replace('example_1','example_1_html5_api').replace(' autoplay=""','')); }); From 998183f6608f6d7499aefa5299e94a07428c4997 Mon Sep 17 00:00:00 2001 From: David Date: Thu, 24 Jul 2014 18:19:18 -0700 Subject: [PATCH 07/13] save original tag attributes --- src/js/player.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/js/player.js b/src/js/player.js index caa2e5d59d..fccb8d4cac 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -32,6 +32,8 @@ vjs.Player = vjs.Component.extend({ init: function(tag, options, ready){ this.tag = tag; // Store the original tag used to set options + this.tagAttributes = (tag) ? vjs.getAttributeValues(tag) : {}; // Store the tag attributes used to restore html5 tech + // Make sure tag ID exists tag.id = tag.id || 'vjs_video_' + vjs.guid++; From a443d214f9a43bd611eeb0acfe10d06f33fdaff6 Mon Sep 17 00:00:00 2001 From: David Date: Thu, 24 Jul 2014 18:19:55 -0700 Subject: [PATCH 08/13] restore original tag attributes n creation and overwrite if required by settings --- src/js/media/html5.js | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/js/media/html5.js b/src/js/media/html5.js index db61fb2b5a..a71942f35d 100644 --- a/src/js/media/html5.js +++ b/src/js/media/html5.js @@ -80,10 +80,13 @@ vjs.Html5.prototype.createEl = function(){ el = clone; player.tag = null; } else { - el = vjs.createEl('video', { - id:player.id() + '_html5_api', - className:'vjs-tech' - }); + el = vjs.createEl('video', {}); + vjs.setElementAttributes(el, + vjs.obj.merge(player.tagAttributes||{}, { + id:player.id() + '_html5_api', + 'class':'vjs-tech' + }) + ); } // associate the player with the new tag el['player'] = player; @@ -95,20 +98,11 @@ vjs.Html5.prototype.createEl = function(){ var attrs = ['autoplay','preload','loop','muted']; for (var i = attrs.length - 1; i >= 0; i--) { var attr = attrs[i]; - if (player.options_[attr] !== null) { - if(typeof player.options_[attr] === 'boolean') { - if(player.options_[attr]) { - el.setAttribute(attr,''); - } else { - el.removeAttribute(attr); - } - } else if(typeof player.options_[attr] === 'undefined') { - el.removeAttribute(attr); - } else { - el.setAttribute(attr,player.options_[attr]); - } - el[attr] = player.options_[attr]; + var attributes = {}; + if (typeof player.options_[attr] !== 'undefined') { + attributes[attr]=player.options_[attr]; } + vjs.setElementAttributes(el, attributes); } return el; From 72bbabddc966d2d293d0f3bd7e78d6e55eb506ac Mon Sep 17 00:00:00 2001 From: David Date: Fri, 25 Jul 2014 16:10:58 -0700 Subject: [PATCH 09/13] replace object.keys with vjs.obj.each for ie<9 --- src/js/lib.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/js/lib.js b/src/js/lib.js index ee87c57888..900e12a007 100644 --- a/src/js/lib.js +++ b/src/js/lib.js @@ -41,16 +41,13 @@ vjs.createEl = function(tagName, properties){ * @private */ vjs.setElementAttributes = function(el, attributes){ - var keys = Object.keys(attributes); - for(var i = 0,l = keys.length;i Date: Fri, 25 Jul 2014 16:12:23 -0700 Subject: [PATCH 10/13] fix spacing --- src/js/media/html5.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/media/html5.js b/src/js/media/html5.js index a71942f35d..22d491fa3d 100644 --- a/src/js/media/html5.js +++ b/src/js/media/html5.js @@ -100,7 +100,7 @@ vjs.Html5.prototype.createEl = function(){ var attr = attrs[i]; var attributes = {}; if (typeof player.options_[attr] !== 'undefined') { - attributes[attr]=player.options_[attr]; + attributes[attr] = player.options_[attr]; } vjs.setElementAttributes(el, attributes); } From b54f5e24811d414c40f87877b669cfbf53008d75 Mon Sep 17 00:00:00 2001 From: David Date: Fri, 25 Jul 2014 16:17:03 -0700 Subject: [PATCH 11/13] API consistency, getAttributeValues renamed to getElementAttributes --- src/js/lib.js | 2 +- src/js/player.js | 8 ++++---- test/unit/lib.js | 10 +++++----- test/unit/player.js | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/js/lib.js b/src/js/lib.js index 900e12a007..288afc9511 100644 --- a/src/js/lib.js +++ b/src/js/lib.js @@ -416,7 +416,7 @@ vjs.TOUCH_ENABLED = !!(('ontouchstart' in window) || window.DocumentTouch && doc * @return {Object} * @private */ -vjs.getAttributeValues = function(tag){ +vjs.getElementAttributes = function(tag){ var obj, knownBooleans, attrs, attrName, attrVal; obj = {}; diff --git a/src/js/player.js b/src/js/player.js index fccb8d4cac..c8a6bf000d 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -32,7 +32,7 @@ vjs.Player = vjs.Component.extend({ init: function(tag, options, ready){ this.tag = tag; // Store the original tag used to set options - this.tagAttributes = (tag) ? vjs.getAttributeValues(tag) : {}; // Store the tag attributes used to restore html5 tech + this.tagAttributes = (tag) ? vjs.getElementAttributes(tag) : {}; // Store the tag attributes used to restore html5 tech // Make sure tag ID exists tag.id = tag.id || 'vjs_video_' + vjs.guid++; @@ -137,7 +137,7 @@ vjs.Player.prototype.getTagSettings = function(tag){ 'tracks': [] }; - vjs.obj.merge(options, vjs.getAttributeValues(tag)); + vjs.obj.merge(options, vjs.getElementAttributes(tag)); // Get tag children settings if (tag.hasChildNodes()) { @@ -150,9 +150,9 @@ vjs.Player.prototype.getTagSettings = function(tag){ // Change case needed: http://ejohn.org/blog/nodename-case-sensitivity/ childName = child.nodeName.toLowerCase(); if (childName === 'source') { - options['sources'].push(vjs.getAttributeValues(child)); + options['sources'].push(vjs.getElementAttributes(child)); } else if (childName === 'track') { - options['tracks'].push(vjs.getAttributeValues(child)); + options['tracks'].push(vjs.getElementAttributes(child)); } } } diff --git a/test/unit/lib.js b/test/unit/lib.js index f4e70ca0ed..a21cc35193 100644 --- a/test/unit/lib.js +++ b/test/unit/lib.js @@ -113,10 +113,10 @@ test('should read tag attributes from elements, including HTML5 in all browsers' document.getElementById('qunit-fixture').innerHTML += tags; - var vid1Vals = vjs.getAttributeValues(document.getElementById('vid1')); - var vid2Vals = vjs.getAttributeValues(document.getElementById('vid2')); - var sourceVals = vjs.getAttributeValues(document.getElementById('source')); - var trackVals = vjs.getAttributeValues(document.getElementById('track')); + var vid1Vals = vjs.getElementAttributes(document.getElementById('vid1')); + var vid2Vals = vjs.getElementAttributes(document.getElementById('vid2')); + var sourceVals = vjs.getElementAttributes(document.getElementById('source')); + var trackVals = vjs.getElementAttributes(document.getElementById('track')); // was using deepEqual, but ie8 would send all properties as attributes @@ -161,7 +161,7 @@ test('should set tag attributes from object', function(){ var el = document.getElementById('vid1'); vjs.setElementAttributes(el, {controls: false,'data-test': 'asdf'}); - var vid1Vals = vjs.getAttributeValues(document.getElementById('vid1')); + var vid1Vals = vjs.getElementAttributes(document.getElementById('vid1')); equal(vid1Vals['controls'], undefined); equal(vid1Vals['id'], 'vid1'); diff --git a/test/unit/player.js b/test/unit/player.js index fac7ced4f3..09d67825db 100644 --- a/test/unit/player.js +++ b/test/unit/player.js @@ -73,7 +73,7 @@ test('should accept options from multiple sources and override in correct order' }); test('should get tag, source, and track settings', function(){ - // Partially tested in lib->getAttributeValues + // Partially tested in lib->getElementAttributes var fixture = document.getElementById('qunit-fixture'); From 1c5f928f5cc37654eb9a655540db129e692303b2 Mon Sep 17 00:00:00 2001 From: David Date: Fri, 25 Jul 2014 16:27:43 -0700 Subject: [PATCH 12/13] clear variable naming --- src/js/media/html5.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/js/media/html5.js b/src/js/media/html5.js index 22d491fa3d..e6cc176e52 100644 --- a/src/js/media/html5.js +++ b/src/js/media/html5.js @@ -95,14 +95,14 @@ vjs.Html5.prototype.createEl = function(){ } // Update specific tag settings, in case they were overridden - var attrs = ['autoplay','preload','loop','muted']; - for (var i = attrs.length - 1; i >= 0; i--) { - var attr = attrs[i]; - var attributes = {}; + var settingsAttrs = ['autoplay','preload','loop','muted']; + for (var i = settingsAttrs.length - 1; i >= 0; i--) { + var attr = settingsAttrs[i]; + var overwriteAttrs = {}; if (typeof player.options_[attr] !== 'undefined') { - attributes[attr] = player.options_[attr]; + overwriteAttrs[attr] = player.options_[attr]; } - vjs.setElementAttributes(el, attributes); + vjs.setElementAttributes(el, overwriteAttrs); } return el; From 0604746f5b4e63366789ae42ffa51542ba6074e6 Mon Sep 17 00:00:00 2001 From: David Date: Fri, 25 Jul 2014 17:01:35 -0700 Subject: [PATCH 13/13] move setElementAttributes close to getElementAttributes --- src/js/lib.js | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/js/lib.js b/src/js/lib.js index 288afc9511..4567926467 100644 --- a/src/js/lib.js +++ b/src/js/lib.js @@ -34,22 +34,6 @@ vjs.createEl = function(tagName, properties){ return el; }; -/** - * Apply attributes to an html element. - * @param {Element} el Target element. - * @param {Object=} attributes Element attributes to be applied. - * @private - */ -vjs.setElementAttributes = function(el, attributes){ - vjs.obj.each(attributes, function(attrName, attrValue) { - if (attrValue === null || typeof attrValue === 'undefined' || attrValue === false) { - el.removeAttribute(attrName); - } else { - el.setAttribute(attrName,attrValue === true ? '' : attrValue); - } - }); -}; - /** * Uppercase the first letter of a string * @param {String} string String to be uppercased @@ -407,6 +391,22 @@ vjs.IS_CHROME = (/Chrome/i).test(vjs.USER_AGENT); vjs.TOUCH_ENABLED = !!(('ontouchstart' in window) || window.DocumentTouch && document instanceof window.DocumentTouch); +/** + * Apply attributes to an HTML element. + * @param {Element} el Target element. + * @param {Object=} attributes Element attributes to be applied. + * @private + */ +vjs.setElementAttributes = function(el, attributes){ + vjs.obj.each(attributes, function(attrName, attrValue) { + if (attrValue === null || typeof attrValue === 'undefined' || attrValue === false) { + el.removeAttribute(attrName); + } else { + el.setAttribute(attrName,attrValue === true ? '' : attrValue); + } + }); +}; + /** * Get an element's attribute values, as defined on the HTML tag * Attributs are not the same as properties. They're defined on the tag