Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix($compile) support templates with table content root nodes
Browse files Browse the repository at this point in the history
If the first element in a template is a <tr>, <th>, <td>, or <tbody> tag,
the HTML compiler will ensure that the template is wrapped in a <table>
element so that the table content is not discarded.

Closes #2848
Closes #1459
Closes #3647
Closes #3241
  • Loading branch information
Caitlin Potter authored and caitp committed Feb 14, 2014
1 parent a9fcb0d commit 31c450b
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 5 deletions.
31 changes: 26 additions & 5 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
var hasDirectives = {},
Suffix = 'Directive',
COMMENT_DIRECTIVE_REGEXP = /^\s*directive\:\s*([\d\w\-_]+)\s+(.*)$/,
CLASS_DIRECTIVE_REGEXP = /(([\d\w\-_]+)(?:\:([^;]+))?;?)/;
CLASS_DIRECTIVE_REGEXP = /(([\d\w\-_]+)(?:\:([^;]+))?;?)/,
TABLE_CONTENT_REGEXP = /^<\s*(tr|th|td|tbody)(\s+[^>]*)?>/i;

// Ref: http://developers.whatwg.org/webappapis.html#event-handler-idl-attributes
// The assumption is that future DOM event attribute names will begin with
Expand Down Expand Up @@ -1243,9 +1244,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {

if (directive.replace) {
replaceDirective = directive;
$template = jqLite('<div>' +
trim(directiveValue) +
'</div>').contents();
$template = directiveTemplateContents(directiveValue);
compileNode = $template[0];

if ($template.length != 1 || compileNode.nodeType !== 1) {
Expand Down Expand Up @@ -1644,6 +1643,28 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}


function directiveTemplateContents(template) {
var type;
template = trim(template);
if ((type = TABLE_CONTENT_REGEXP.exec(template))) {
type = type[1].toLowerCase();
var table = jqLite('<table>' + template + '</table>'),
tbody = table.children('tbody'),
leaf = /(td|th)/.test(type) && table.find('tr');
if (tbody.length && type !== 'tbody') {
table = tbody;
}
if (leaf && leaf.length) {
table = leaf;
}
return table.contents();
}
return jqLite('<div>' +
template +
'</div>').contents();
}


function compileTemplateUrl(directives, $compileNode, tAttrs,
$rootElement, childTranscludeFn, preLinkFns, postLinkFns, previousCompileContext) {
var linkQueue = [],
Expand All @@ -1668,7 +1689,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
content = denormalizeTemplate(content);

if (origAsyncDirective.replace) {
$template = jqLite('<div>' + trim(content) + '</div>').contents();
$template = directiveTemplateContents(content);
compileNode = $template[0];

if ($template.length != 1 || compileNode.nodeType !== 1) {
Expand Down
97 changes: 97 additions & 0 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,22 @@ describe('$compile', function() {
expect(element).toBe(attr.$$element);
}
}));
directive('replaceWithTr', valueFn({
replace: true,
template: '<tr><td>TR</td></tr>'
}));
directive('replaceWithTd', valueFn({
replace: true,
template: '<td>TD</td>'
}));
directive('replaceWithTh', valueFn({
replace: true,
template: '<th>TH</th>'
}));
directive('replaceWithTbody', valueFn({
replace: true,
template: '<tbody><tr><td>TD</td></tr></tbody>'
}));
}));


Expand Down Expand Up @@ -680,6 +696,34 @@ describe('$compile', function() {
}).not.toThrow();
});
});

it('should support templates with root <tr> tags', inject(function($compile, $rootScope) {
expect(function() {
element = $compile('<div replace-with-tr></div>')($rootScope);
}).not.toThrow();
expect(nodeName_(element)).toMatch(/tr/i);
}));

it('should support templates with root <td> tags', inject(function($compile, $rootScope) {
expect(function() {
element = $compile('<div replace-with-td></div>')($rootScope);
}).not.toThrow();
expect(nodeName_(element)).toMatch(/td/i);
}));

it('should support templates with root <th> tags', inject(function($compile, $rootScope) {
expect(function() {
element = $compile('<div replace-with-th></div>')($rootScope);
}).not.toThrow();
expect(nodeName_(element)).toMatch(/th/i);
}));

it('should support templates with root <tbody> tags', inject(function($compile, $rootScope) {
expect(function() {
element = $compile('<div replace-with-tbody></div>')($rootScope);
}).not.toThrow();
expect(nodeName_(element)).toMatch(/tbody/i);
}));
});


Expand Down Expand Up @@ -776,6 +820,23 @@ describe('$compile', function() {
replace: true,
template: '<span>Hello, {{name}}!</span>'
}));

directive('replaceWithTr', valueFn({
replace: true,
templateUrl: 'tr.html'
}));
directive('replaceWithTd', valueFn({
replace: true,
templateUrl: 'td.html'
}));
directive('replaceWithTh', valueFn({
replace: true,
templateUrl: 'th.html'
}));
directive('replaceWithTbody', valueFn({
replace: true,
templateUrl: 'tbody.html'
}));
}
));

Expand Down Expand Up @@ -1411,6 +1472,42 @@ describe('$compile', function() {
expect(element.html()).toContain('i = 1');
});
});

it('should support templates with root <tr> tags', inject(function($compile, $rootScope, $templateCache) {
$templateCache.put('tr.html', '<tr><td>TR</td></tr>');
expect(function() {
element = $compile('<div replace-with-tr></div>')($rootScope);
}).not.toThrow();
$rootScope.$digest();
expect(nodeName_(element)).toMatch(/tr/i);
}));

it('should support templates with root <td> tags', inject(function($compile, $rootScope, $templateCache) {
$templateCache.put('td.html', '<td>TD</td>');
expect(function() {
element = $compile('<div replace-with-td></div>')($rootScope);
}).not.toThrow();
$rootScope.$digest();
expect(nodeName_(element)).toMatch(/td/i);
}));

it('should support templates with root <th> tags', inject(function($compile, $rootScope, $templateCache) {
$templateCache.put('th.html', '<th>TH</th>');
expect(function() {
element = $compile('<div replace-with-th></div>')($rootScope);
}).not.toThrow();
$rootScope.$digest();
expect(nodeName_(element)).toMatch(/th/i);
}));

it('should support templates with root <tbody> tags', inject(function($compile, $rootScope, $templateCache) {
$templateCache.put('tbody.html', '<tbody><tr><td>TD</td></tr></tbody>');
expect(function() {
element = $compile('<div replace-with-tbody></div>')($rootScope);
}).not.toThrow();
$rootScope.$digest();
expect(nodeName_(element)).toMatch(/tbody/i);
}));
});


Expand Down

1 comment on commit 31c450b

@caitp
Copy link
Contributor

@caitp caitp commented on 31c450b Feb 14, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit also closed PR #5235

Please sign in to comment.