Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fixes #2127 #2166

Merged
merged 5 commits into from

2 participants

@ibolmo
Owner

Used sIEve to track leaks in Element.js. @gd0t claims that the problem
is in Array.flatten, but I coudn't reproduce. I did find, however, that
we had various onload/unload leaks in Element.js. Most where feature
feature detects that were not nulled after usage.

This PR also includes improvements (and fixes) for set('html', ...).

@ibolmo
Owner

Fixes #2127.

Source/Element/Element.js
((7 lines not shown))
var x = document.createElement('<input name=x>');
- createElementAcceptsHTML = (x.name == 'x');
-} catch(e){}
+ return (x.name == 'x');
@arian Owner
arian added a note

why not just

return document.createElement('<input name=x>').name == 'x';
@arian Owner
arian added a note

or

var createElementAcceptsHTML;
try {
    createElementAcceptsHTML = (document.createElement('<input name=x>').name == 'x');
} catch (e){}

that seems to work as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Source/Element/Element.js
((82 lines not shown))
- return html;
-})();
-/*</!webkit>*/
+/*<ltFF4>*/
+var tr = document.createElement('tr'), html = '<td></td>';
+tr.innerHTML = html;
+var supportsTRInnerHTML = (tr.innerHTML == html);
+/*</ltFF4>*/
+
+if (!supportsTableInnerHTML || !supportsTRInnerHTML){
@arian Owner
arian added a note

Should also include:

|| !supportsHTML5Elements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@arian arian commented on the diff
Source/Element/Element.js
((82 lines not shown))
- return html;
-})();
-/*</!webkit>*/
+/*<ltFF4>*/
+var tr = document.createElement('tr'), html = '<td></td>';
+tr.innerHTML = html;
+var supportsTRInnerHTML = (tr.innerHTML == html);
@arian Owner
arian added a note

no tr = null here, like the div = null above?

@arian Owner
arian added a note

Yes, I just tried sIEve myself and reported this TR element, using tr = null fixed that.

@ibolmo Owner
ibolmo added a note
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
ibolmo added some commits
@ibolmo ibolmo Fixes 2127.
Used sIEve to track leaks in Element.js. @gd0t claims that the problem
is in Array.flatten, but I coudn't reproduce. I did find, however, that
we had various onload/unload leaks in Element.js. Most where feature
feature detects that were not nulled after usage.

Tip: Use Function.attempt. Looks like Node references in closures are
collected at the end of the closure. See diff.

UNTESTED (next commit).
626aae4
@ibolmo ibolmo Fix specs and refactoring.
Fixed a bug .get('html') bug introduced in the last commit.
Fixed a legacy bug that always tried to fix innerHTML for some elements.
This required a bit of refactoring of the code, so please review
carefully.

PASSED: IE6-9; FFx 3-5, 8, 10; Chrome latest; Safari 5; Opera 11
b317c97
@ibolmo ibolmo Aesthetic fixes, as per arian. abedec7
@ibolmo ibolmo Fixing erase('html') 1ebbd44
@ibolmo ibolmo Including arian's fixes/comments. 73b3b17
@arian arian merged commit a64bcbd into mootools:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 9, 2012
  1. @ibolmo

    Fixes 2127.

    ibolmo authored
    Used sIEve to track leaks in Element.js. @gd0t claims that the problem
    is in Array.flatten, but I coudn't reproduce. I did find, however, that
    we had various onload/unload leaks in Element.js. Most where feature
    feature detects that were not nulled after usage.
    
    Tip: Use Function.attempt. Looks like Node references in closures are
    collected at the end of the closure. See diff.
    
    UNTESTED (next commit).
  2. @ibolmo

    Fix specs and refactoring.

    ibolmo authored
    Fixed a bug .get('html') bug introduced in the last commit.
    Fixed a legacy bug that always tried to fix innerHTML for some elements.
    This required a bit of refactoring of the code, so please review
    carefully.
    
    PASSED: IE6-9; FFx 3-5, 8, 10; Chrome latest; Safari 5; Opera 11
  3. @ibolmo

    Aesthetic fixes, as per arian.

    ibolmo authored
  4. @ibolmo

    Fixing erase('html')

    ibolmo authored
  5. @ibolmo
This page is out of date. Refresh to see the latest.
Showing with 102 additions and 60 deletions.
  1. +71 −60 Source/Element/Element.js
  2. +31 −0 Specs/1.4client/Element/Element.adopt.html
View
131 Source/Element/Element.js
@@ -201,9 +201,8 @@ Array.mirror(Elements);
/*<ltIE8>*/
var createElementAcceptsHTML;
try {
- var x = document.createElement('<input name=x>');
- createElementAcceptsHTML = (x.name == 'x');
-} catch(e){}
+ createElementAcceptsHTML = (document.createElement('<input name=x>').name == 'x');
+} catch (e){}
var escapeQuotes = function(html){
return ('' + html).replace(/&/g, '&amp;').replace(/"/g, '&quot;');
@@ -519,13 +518,8 @@ Array.forEach([
properties[property.toLowerCase()] = property;
});
-Object.append(properties, {
- 'html': 'innerHTML',
- 'text': (function(){
- var temp = document.createElement('div');
- return (temp.textContent == null) ? 'innerText': 'textContent';
- })()
-});
+properties.html = 'innerHTML';
+properties.text = (document.createElement('div').textContent == null) ? 'innerText': 'textContent';
Object.forEach(properties, function(real, key){
propertySetters[key] = function(node, value){
@@ -590,6 +584,7 @@ try { el.type = 'button'; } catch(e){}
if (el.type != 'button') propertySetters.type = function(node, value){
node.setAttribute('type', value);
};
+el = null;
/* </webkit> */
/* getProperty, setProperty */
@@ -912,61 +907,77 @@ Element.Properties.tag = {
};
-/*<!webkit>*/
-Element.Properties.html = (function(){
-
- var tableTest = Function.attempt(function(){
- var table = document.createElement('table');
- table.innerHTML = '<tr><td></td></tr>';
- });
+Element.Properties.html = {
- var wrapper = document.createElement('div');
+ set: function(html){
+ if (html == null) html = '';
+ else if (typeOf(html) == 'array') html = html.join('');
+ this.innerHTML = html;
+ },
- var translations = {
- table: [1, '<table>', '</table>'],
- select: [1, '<select>', '</select>'],
- tbody: [2, '<table><tbody>', '</tbody></table>'],
- tr: [3, '<table><tbody><tr>', '</tr></tbody></table>']
- };
- translations.thead = translations.tfoot = translations.tbody;
-
- /*<ltIE9>*/
- // technique by jdbarlett - http://jdbartlett.com/innershiv/
- wrapper.innerHTML = '<nav></nav>';
- var HTML5Test = wrapper.childNodes.length == 1;
- if (!HTML5Test){
- var tags = 'abbr article aside audio canvas datalist details figcaption figure footer header hgroup mark meter nav output progress section summary time video'.split(' '),
- fragment = document.createDocumentFragment(), l = tags.length;
- while (l--) fragment.createElement(tags[l]);
- fragment.appendChild(wrapper);
+ erase: function(){
+ this.innerHTML = '';
}
- /*</ltIE9>*/
- var html = {
- set: function(html){
- if (typeOf(html) == 'array') html = html.join('');
- else if (html == null) html = '';
+};
- var wrap = (!tableTest && translations[this.get('tag')]);
- /*<ltIE9>*/
- if (!wrap && !HTML5Test) wrap = [0, '', ''];
- /*</ltIE9>*/
- if (wrap){
- var first = wrapper;
- first.innerHTML = wrap[1] + html + wrap[2];
- for (var i = wrap[0]; i--;) first = first.firstChild;
- this.empty().adopt(first.childNodes);
- } else {
- this.innerHTML = html;
- }
- }
- };
+/*<ltIE9>*/
+// technique by jdbarlett - http://jdbartlett.com/innershiv/
+var div = document.createElement('div');
+div.innerHTML = '<nav></nav>';
+var supportsHTML5Elements = (div.childNodes.length == 1);
+if (!supportsHTML5Elements){
+ var tags = 'abbr article aside audio canvas datalist details figcaption figure footer header hgroup mark meter nav output progress section summary time video'.split(' '),
+ fragment = document.createDocumentFragment(), l = tags.length;
+ while (l--) fragment.createElement(tags[l]);
+}
+div = null;
+/*</ltIE9>*/
- html.erase = html.set;
+/*<IE>*/
+var supportsTableInnerHTML = Function.attempt(function(){
+ var table = document.createElement('table');
+ table.innerHTML = '<tr><td></td></tr>';
+ return true;
+});
- return html;
-})();
-/*</!webkit>*/
+/*<ltFF4>*/
+var tr = document.createElement('tr'), html = '<td></td>';
+tr.innerHTML = html;
+var supportsTRInnerHTML = (tr.innerHTML == html);
@arian Owner
arian added a note

no tr = null here, like the div = null above?

@arian Owner
arian added a note

Yes, I just tried sIEve myself and reported this TR element, using tr = null fixed that.

@ibolmo Owner
ibolmo added a note
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+tr = null;
+/*</ltFF4>*/
+
+if (!supportsTableInnerHTML || !supportsTRInnerHTML || !supportsHTML5Elements){
+
+ Element.Properties.html.set = (function(set){
+
+ var translations = {
+ table: [1, '<table>', '</table>'],
+ select: [1, '<select>', '</select>'],
+ tbody: [2, '<table><tbody>', '</tbody></table>'],
+ tr: [3, '<table><tbody><tr>', '</tr></tbody></table>']
+ };
+
+ translations.thead = translations.tfoot = translations.tbody;
+
+ return function(html){
+ var wrap = translations[this.get('tag')];
+ if (!wrap && !supportsHTML5Elements) wrap = [0, '', ''];
+ if (!wrap) return set.call(this, html);
+
+ var level = wrap[0], wrapper = document.createElement('div'), target = wrapper;
+ if (!supportsHTML5Elements) fragment.appendChild(wrapper);
+ wrapper.innerHTML = [wrap[1], html, wrap[2]].flatten().join('');
+ while (level--) target = target.firstChild;
+ this.empty().adopt(target.childNodes);
+ if (!supportsHTML5Elements) fragment.removeChild(wrapper);
+ wrapper = null;
+ };
+
+ })(Element.Properties.html.set);
+}
+/*</IE>*/
/*<ltIE9>*/
var testForm = document.createElement('form');
@@ -998,11 +1009,11 @@ if (testForm.firstChild.value != 's') Element.Properties.value = {
}
};
+testForm = null;
/*</ltIE9>*/
/*<IE>*/
-var el = document.createElement('div');
-if (el.getAttributeNode('id')) Element.Properties.id = {
+if (document.createElement('div').getAttributeNode('id')) Element.Properties.id = {
set: function(id){
this.id = this.getAttributeNode('id').value = id;
},
View
31 Specs/1.4client/Element/Element.adopt.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <meta http-equiv="Content-Type" content="text/html;charset=UTF-8" />
+
+ <title>Element Adopt Leak Test</title>
+
+ <script src="../../../Source/Core/Core.js" type="text/javascript"></script>
+ <script src="../../../Source/Types/Array.js" type="text/javascript"></script>
+ <script src="../../../Source/Types/Function.js" type="text/javascript"></script>
+ <script src="../../../Source/Types/Number.js" type="text/javascript"></script>
+ <script src="../../../Source/Types/String.js" type="text/javascript"></script>
+ <script src="../../../Source/Types/Object.js" type="text/javascript"></script>
+ <script src="../../../Source/Types/DOMEvent.js" type="text/javascript"></script>
+ <script src="../../../Source/Browser/Browser.js" type="text/javascript"></script>
+ <script src="../../../Source/Slick/Slick.Parser.js" type="text/javascript"></script>
+ <script src="../../../Source/Slick/Slick.Finder.js" type="text/javascript"></script>
+ <script src="../../../Source/Element/Element.js" type="text/javascript"></script>
+ <script src="../../../Source/Element/Element.Event.js" type="text/javascript"></script>
+
+ <body>
+
+ <script>
+ window.addEvent('domready', function(){
+ var newDiv = new Element('div', { id: 'newrDiv' });
+ $(document.body).adopt(newDiv);
+ });
+ </script>
+ </body>
+</html>
+
Something went wrong with that request. Please try again.