mirror of
https://github.com/meteor/meteor.git
synced 2026-05-02 03:01:46 -04:00
Kill TBODY special case in DomRange
Background: When browsers parse HTML, they always insert a <tbody> around your <tr>s if you don't have one. That is, `<table><tr>` becomes `<table><tbody><tr>`. However, if you use the DOM API (appendChild et al.), this doesn't happen. According to the latest specs and modern browsers, the TBODY element is not necessary in the DOM (though it is still always inserted when parsing HTML). IE 6/7 disagrees, but that's history. jQuery bends over backwards to insert TBODY elements dynamically, whenever you put a TR inside a TABLE. Previously, we did too, but it's even harder for us.
This commit removes the code that special-cases tables in DomRange. The code was never completely finished, and it will slow us down to have the extra essential complexity in DomRange. Now, when you write `<table><tr>`, you'll get `<table><tr>` in the DOM tree.
It's important that we never optimize `<table><tr>...` into innerHTML, or else a template like `<table><tr><td>Foo</td></tr></table>` will get an inserted TBODY, while the same template with `{{foo}}` instead of `Foo` won't (because the table can't be optimized into static HTML). A new test checks that we do this right.
The main downside of this change is that jQuery isn't as good for manipulating tables created by Meteor. However, I think we can live with that. jQuery made a particular choice there, and any problems that come up will be consistent across browsers and have workarounds (like inserting a <tbody> or not using a <table> tag).
Another downside concerns server-side rendering. Rendering a template to a complete HTML string containing `<table><tr>` and then splatting that into the page will produce <tbody> tags that wouldn't be there if the page were rendered on the client. We'll cross that bridge when we come to it. In most ways (e.g. CSS rules), it shouldn't matter if there are <tbody> tags or not.
This commit is contained in:
David Greenspan
@@ -29,6 +29,10 @@ var optimize = function (tree) {
|
||||
return (html.indexOf('&') < 0 && html.indexOf('<') < 0);
|
||||
};
|
||||
|
||||
// Returns `null` if no specials are found in the array, so that the
|
||||
// parent can perform the actual optimization. Otherwise, returns
|
||||
// an array of parts which have been optimized as much as possible.
|
||||
// `forceOptimize` forces the latter case.
|
||||
var optimizeArrayParts = function (array, optimizePartsFunc, forceOptimize) {
|
||||
var result = null;
|
||||
if (forceOptimize)
|
||||
@@ -102,7 +106,14 @@ var optimize = function (tree) {
|
||||
|
||||
var mustOptimize = false;
|
||||
|
||||
if (node.attrs) {
|
||||
// Avoid ever producing HTML containing `<table><tr>...`, because the
|
||||
// browser will insert a TBODY. If we just `createElement("table")` and
|
||||
// `createElement("tr")`, on the other hand, no TBODY is necessary
|
||||
// (assuming IE 8+).
|
||||
if (tagName === 'table')
|
||||
mustOptimize = true;
|
||||
|
||||
if (node.attrs && ! mustOptimize) {
|
||||
var attrs = node.attrs;
|
||||
for (var k in attrs) {
|
||||
if (doesAttributeValueHaveSpecials(attrs[k])) {
|
||||
|
||||
@@ -663,3 +663,11 @@ Hi there!
|
||||
<template name="spacebars_test_event_returns_false">
|
||||
<a href="#bad-url" id="spacebars_test_event_returns_false_link">click me</a>
|
||||
</template>
|
||||
|
||||
<template name="spacebars_test_tables1">
|
||||
<table><tr><td>Foo</td></tr></table>
|
||||
</template>
|
||||
|
||||
<template name="spacebars_test_tables2">
|
||||
<table><tr><td>{{foo}}</td></tr></table>
|
||||
</template>
|
||||
|
||||
@@ -1832,3 +1832,19 @@ Tinytest.add(
|
||||
document.body.removeChild(div);
|
||||
}
|
||||
);
|
||||
|
||||
Tinytest.add("spacebars - template - tables", function (test) {
|
||||
var tmpl1 = Template.spacebars_test_tables1;
|
||||
|
||||
var div = renderToDiv(tmpl1);
|
||||
test.equal(_.pluck(div.querySelectorAll('*'), 'tagName'),
|
||||
['TABLE', 'TR', 'TD']);
|
||||
divRendersTo(test, div, '<table><tr><td>Foo</td></tr></table>');
|
||||
|
||||
var tmpl2 = Template.spacebars_test_tables2;
|
||||
tmpl2.foo = 'Foo';
|
||||
div = renderToDiv(tmpl2);
|
||||
test.equal(_.pluck(div.querySelectorAll('*'), 'tagName'),
|
||||
['TABLE', 'TR', 'TD']);
|
||||
divRendersTo(test, div, '<table><tr><td>Foo</td></tr></table>');
|
||||
});
|
||||
|
||||
@@ -3,7 +3,6 @@
|
||||
// - UI hooks (expose, test)
|
||||
// - Quick remove/add (mark "leaving" members; needs UI hooks)
|
||||
// - Event removal on removal
|
||||
// - Event moving on TBODY move
|
||||
|
||||
var DomBackend = UI.DomBackend;
|
||||
|
||||
@@ -98,9 +97,6 @@ var rangeParented = function (range) {
|
||||
// element. This is really just for IE 9+
|
||||
// TextNode GC issues, but we can't do reliable
|
||||
// feature detection (i.e. bug detection).
|
||||
// Note that because we keep a direct pointer to
|
||||
// `parentNode.$_uiranges`, it doesn't matter
|
||||
// if we are reparented (e.g. wrapped in a TBODY).
|
||||
var parentNode = range.parentNode();
|
||||
var rangeDict = (
|
||||
parentNode.$_uiranges ||
|
||||
@@ -306,11 +302,6 @@ _extend(DomRange.prototype, {
|
||||
range.owner = this;
|
||||
var nodes = range.getNodes();
|
||||
|
||||
if (tbodyFixNeeded(nodes, parentNode))
|
||||
// may cause a refresh(); important that the
|
||||
// member isn't added yet
|
||||
parentNode = moveWithOwnersIntoTbody(this);
|
||||
|
||||
members[id] = newMember;
|
||||
for (var i = 0; i < nodes.length; i++)
|
||||
insertNode(nodes[i], parentNode, nextNode);
|
||||
@@ -327,11 +318,6 @@ _extend(DomRange.prototype, {
|
||||
if (node.nodeType !== 3)
|
||||
node.$ui = this;
|
||||
|
||||
if (tbodyFixNeeded(node, parentNode))
|
||||
// may cause a refresh(); important that the
|
||||
// member isn't added yet
|
||||
parentNode = moveWithOwnersIntoTbody(this);
|
||||
|
||||
members[id] = newMember;
|
||||
insertNode(node, parentNode, nextNode);
|
||||
}
|
||||
@@ -720,8 +706,6 @@ DomRange.getComponents = function (element) {
|
||||
// `parentNode` must be an ELEMENT, not a fragment
|
||||
DomRange.insert = function (range, parentNode, nextNode) {
|
||||
var nodes = range.getNodes();
|
||||
if (tbodyFixNeeded(nodes, parentNode))
|
||||
parentNode = makeOrFindTbody(parentNode, nextNode);
|
||||
for (var i = 0; i < nodes.length; i++)
|
||||
insertNode(nodes[i], parentNode, nextNode);
|
||||
rangeParented(range);
|
||||
@@ -741,65 +725,6 @@ DomRange.getContainingComponent = function (element) {
|
||||
return null;
|
||||
};
|
||||
|
||||
///// TBODY FIX for compatibility with jQuery.
|
||||
//
|
||||
// Because people might use jQuery from UI hooks, and
|
||||
// jQuery is unable to do $(myTable).append(myTR) without
|
||||
// adding a TBODY (for historical reasons), we move any DomRange
|
||||
// that gains a TR, and its immediately enclosing DomRanges,
|
||||
// into a TBODY.
|
||||
//
|
||||
// See http://www.quora.com/David-Greenspan/Posts/The-Great-TBODY-Debacle
|
||||
var tbodyFixNeeded = function (childOrChildren, parent) {
|
||||
if (parent.nodeName !== 'TABLE')
|
||||
return false;
|
||||
|
||||
if (isArray(childOrChildren)) {
|
||||
var foundTR = false;
|
||||
for (var i = 0, N = childOrChildren.length; i < N; i++) {
|
||||
var n = childOrChildren[i];
|
||||
if (n.nodeType === 1 && n.nodeName === 'TR') {
|
||||
foundTR = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (! foundTR)
|
||||
return false;
|
||||
} else {
|
||||
var n = childOrChildren;
|
||||
if (! (n.nodeType === 1 && n.nodeName === 'TR'))
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
};
|
||||
|
||||
var makeOrFindTbody = function (parent, next) {
|
||||
// we have a TABLE > TR situation
|
||||
var tbody = parent.getElementsByTagName('tbody')[0];
|
||||
if (! tbody) {
|
||||
tbody = parent.ownerDocument.createElement("tbody");
|
||||
parent.insertBefore(tbody, next || null);
|
||||
}
|
||||
return tbody;
|
||||
};
|
||||
|
||||
var moveWithOwnersIntoTbody = function (range) {
|
||||
while (range.owner)
|
||||
range = range.owner;
|
||||
|
||||
var nodes = range.getNodes(); // causes refresh
|
||||
var tbody = makeOrFindTbody(range.parentNode(),
|
||||
range.end.nextSibling);
|
||||
for (var i = 0; i < nodes.length; i++)
|
||||
tbody.appendChild(nodes[i]);
|
||||
|
||||
// XXX complete the reparenting by moving event
|
||||
// HandlerRecs of `range`.
|
||||
|
||||
return tbody;
|
||||
};
|
||||
|
||||
///// FIND BY SELECTOR
|
||||
|
||||
DomRange.prototype.contains = function (compOrNode) {
|
||||
|
||||
@@ -489,133 +489,6 @@ Tinytest.add("ui - DomRange - external moves", function (test) {
|
||||
strip('-------------- (aXb ghiWjkl)'));
|
||||
});
|
||||
|
||||
Tinytest.add("ui - DomRange - tables", function (test) {
|
||||
var range = function (x) {
|
||||
// create a range x.dom containing an element x.el,
|
||||
// inside that element, the range x.content.dom
|
||||
x.dom = new DomRange;
|
||||
if (x.el) {
|
||||
x.dom.add(x.el);
|
||||
if (x.content)
|
||||
DomRange.insert(x.content.dom, x.el);
|
||||
}
|
||||
return x;
|
||||
};
|
||||
var tr, td;
|
||||
var table = range({
|
||||
el: document.createElement('table'),
|
||||
content: tr = range({
|
||||
el: document.createElement('tr'),
|
||||
content: td = range({
|
||||
el: document.createElement('td')
|
||||
})
|
||||
})
|
||||
});
|
||||
|
||||
// TBODY got inserted automatically.
|
||||
// This tests DomRange.insert.
|
||||
test.equal(table.el.childNodes.length, 1);
|
||||
test.equal(table.el.firstChild.nodeName, 'TBODY');
|
||||
// TBODY contains [start, TR, end]
|
||||
test.equal(table.el.firstChild.childNodes.length, 3);
|
||||
test.equal(table.el.firstChild.childNodes[1], tr.el);
|
||||
test.equal(tr.el.childNodes.length, 3);
|
||||
test.equal(tr.el.childNodes[1], td.el);
|
||||
|
||||
// start over
|
||||
$(table.el).empty();
|
||||
test.equal(table.el.childNodes.length, 0);
|
||||
|
||||
table.content = range({});
|
||||
DomRange.insert(table.content.dom, table.el);
|
||||
// table has two children (start/end markers), no elements
|
||||
test.equal(table.el.childNodes.length, 2);
|
||||
test.notEqual(table.el.firstChild.nodeType, 1);
|
||||
test.notEqual(table.el.lastChild.nodeType, 1);
|
||||
|
||||
// shazam, adding a TR should move the whole range
|
||||
// into a TBODY. This tests range.add(node).
|
||||
table.content.dom.add(document.createElement('tr'));
|
||||
|
||||
test.equal(table.el.childNodes.length, 1);
|
||||
test.equal(table.el.firstChild.nodeName, 'TBODY');
|
||||
test.equal(table.el.firstChild.childNodes.length, 3);
|
||||
test.equal(table.el.firstChild.childNodes[1].nodeName, 'TR');
|
||||
|
||||
// start over.
|
||||
$(table.el).empty();
|
||||
test.equal(table.el.childNodes.length, 0);
|
||||
|
||||
table.content = range({});
|
||||
DomRange.insert(table.content.dom, table.el);
|
||||
var a1 = range({});
|
||||
var a2 = range({});
|
||||
a1.dom.add(a2.dom);
|
||||
table.content.dom.add(a1.dom);
|
||||
// 6 marker nodes in table, no elements
|
||||
test.equal(table.el.childNodes.length, 6);
|
||||
test.equal($(table.el).find("*").length, 0);
|
||||
// shazam, adding a TR to the innermost range
|
||||
// should move all the ranges into a TBODY.
|
||||
a2.dom.add(document.createElement('tr'));
|
||||
test.equal(table.el.childNodes.length, 1);
|
||||
test.equal(table.el.firstChild.nodeName, 'TBODY');
|
||||
test.equal(table.el.firstChild.childNodes.length, 7);
|
||||
test.equal(table.el.firstChild.childNodes[3].nodeName, 'TR');
|
||||
|
||||
// start over. this time test adding a range containing
|
||||
// a TR.
|
||||
$(table.el).empty();
|
||||
test.equal(table.el.childNodes.length, 0);
|
||||
|
||||
table.content = range({});
|
||||
DomRange.insert(table.content.dom, table.el);
|
||||
var b1 = range({});
|
||||
var b2 = range({});
|
||||
table.content.dom.add(b1.dom);
|
||||
b2.dom.add(document.createElement('tr'));
|
||||
// 4 marker nodes in table, no elements
|
||||
test.equal(table.el.childNodes.length, 4);
|
||||
test.equal($(table.el).find("*").length, 0);
|
||||
// shazam, adding b2, which contains a TR,
|
||||
// should move all the ranges into a TBODY.
|
||||
b1.dom.add(b2.dom);
|
||||
test.equal(table.el.childNodes.length, 1);
|
||||
test.equal(table.el.firstChild.nodeName, 'TBODY');
|
||||
test.equal(table.el.firstChild.childNodes.length, 7);
|
||||
test.equal(table.el.firstChild.childNodes[3].nodeName, 'TR');
|
||||
|
||||
test.equal(b2.dom.parentNode().nodeName, 'TBODY');
|
||||
test.equal(b1.dom.parentNode().nodeName, 'TBODY');
|
||||
test.equal(table.content.dom.parentNode().nodeName, 'TBODY');
|
||||
|
||||
|
||||
// start over. now test two TR ranges.
|
||||
$(table.el).empty();
|
||||
test.equal(table.el.childNodes.length, 0);
|
||||
|
||||
var c1 = range({});
|
||||
var c2 = range({});
|
||||
DomRange.insert(c1.dom, table.el);
|
||||
DomRange.insert(c2.dom, table.el);
|
||||
test.equal(table.el.childNodes.length, 4);
|
||||
test.equal($(table.el).find("*").length, 0);
|
||||
c2.dom.add(document.createElement('tr'));
|
||||
test.equal(table.el.childNodes.length, 3);
|
||||
test.equal($(table.el).find("> *").length, 1);
|
||||
test.equal($(table.el).find("> tbody").length, 1);
|
||||
c1.dom.add(document.createElement('tr'));
|
||||
// now there should be a single TBODY with two
|
||||
// ranges in it containing TRs
|
||||
test.equal(table.el.childNodes.length, 1);
|
||||
test.equal(table.el.firstChild.nodeName, 'TBODY');
|
||||
var tbody = table.el.firstChild;
|
||||
test.equal(tbody.childNodes.length, 6);
|
||||
test.equal($(tbody).find("> *").length, 2); // 2 elements
|
||||
test.equal(tbody.childNodes[1].nodeName, 'TR');
|
||||
test.equal(tbody.childNodes[4].nodeName, 'TR');
|
||||
});
|
||||
|
||||
Tinytest.add("ui - DomRange - basic events", function (test) {
|
||||
// test.equal doesn't work on arrays of DOM nodes, so
|
||||
// we need this. It's `===` that descends recursively
|
||||
|
||||
Reference in New Issue
Block a user