Skip to content

Commit

Permalink
[BUGFIX beta] contextualElements, handle omitted start tags
Browse files Browse the repository at this point in the history
During a renderTree call, look to the morphs for information about the
contextualElement. Pass this to the Ember renderer, which in turn passes
it to the buffer for use when generating an element.

Here I've changed the morph creation points to explicitly pass a
contextualElement. This avoids the possibility of createMorph's fallback
to document.body happening.

Additionally, strip omitted start tags even if the child node causing
their generation is after a script tag. See the long comment in
`render_buffer`.
  • Loading branch information
mixonic committed Sep 14, 2014
1 parent dfdf309 commit 9cb03e8
Show file tree
Hide file tree
Showing 9 changed files with 253 additions and 30 deletions.
23 changes: 19 additions & 4 deletions packages/ember-metal-views/lib/renderer.js
Expand Up @@ -30,8 +30,12 @@ function Renderer_renderTree(_view, _parentView, _insertAt) {
var parentIndex = -1;
var parents = this._parents;
var parent = null;
if (_parentView) {
parent = _parentView;
}
var elements = this._elements;
var element = null;
var contextualElement = null;
var level = 0;

var view = _view;
Expand All @@ -50,7 +54,18 @@ function Renderer_renderTree(_view, _parentView, _insertAt) {
}

this.willCreateElement(view);
element = this.createElement(view);

contextualElement = view._morph && view._morph.contextualElement;
if (!contextualElement && parent && parent._childViewsMorph) {
contextualElement = parent._childViewsMorph.contextualElement;
}
if (!contextualElement && view._didCreateElementWithoutMorph) {
// This code path is only used by createElement and rerender when createElement
// was previously called on a view.
contextualElement = document.body;
}
Ember.assert("Required contextualElement for view "+_view+" is missing", contextualElement);
element = this.createElement(view, contextualElement);

parents[level++] = parentIndex;
parentIndex = index;
Expand Down Expand Up @@ -87,7 +102,7 @@ function Renderer_renderTree(_view, _parentView, _insertAt) {
}

parentIndex = parents[level];
parent = views[parentIndex];
parent = parentIndex === -1 ? _parentView : views[parentIndex];
this.insertElement(view, parent, element, -1);
index = queue[--length];
view = views[index];
Expand Down Expand Up @@ -132,13 +147,13 @@ Renderer.prototype.scheduleInsert =

Renderer.prototype.appendTo =
function Renderer_appendTo(view, target) {
var morph = this._dom.appendMorph(target);
var morph = this._dom.appendMorph(target, target);
this.scheduleInsert(view, morph);
};

Renderer.prototype.replaceIn =
function Renderer_replaceIn(view, target) {
var morph = this._dom.createMorph(target, null, null);
var morph = this._dom.createMorph(target, null, null, target);
this.scheduleInsert(view, morph);
};

Expand Down
18 changes: 18 additions & 0 deletions packages/ember-metal-views/tests/main_test.js
Expand Up @@ -11,13 +11,15 @@ testsFor("ember-metal-views", {
}
});

// Test the behavior of the helper createElement stub
test("by default, view renders as a div", function() {
view = {isView: true};

appendTo(view);
equalHTML('qunit-fixture', "<div></div>");
});

// Test the behavior of the helper createElement stub
test("tagName can be specified", function() {
view = {
isView: true,
Expand All @@ -29,6 +31,7 @@ test("tagName can be specified", function() {
equalHTML('qunit-fixture', "<span></span>");
});

// Test the behavior of the helper createElement stub
test("textContent can be specified", function() {
view = {
isView: true,
Expand All @@ -40,6 +43,7 @@ test("textContent can be specified", function() {
equalHTML('qunit-fixture', "<div>ohai &lt;a&gt;derp&lt;/a&gt;</div>");
});

// Test the behavior of the helper createElement stub
test("innerHTML can be specified", function() {
view = {
isView: true,
Expand All @@ -51,6 +55,20 @@ test("innerHTML can be specified", function() {
equalHTML('qunit-fixture', "<div>ohai <a>derp</a></div>");
});

// Test the behavior of the helper createElement stub
test("innerHTML tr can be specified", function() {
view = {
isView: true,
tagName: 'table',
innerHTML: '<tr><td>ohai</td></tr>'
};

appendTo(view);

equalHTML('qunit-fixture', "<table><tbody><tr><td>ohai</td></tr></tbody></table>");
});

// Test the behavior of the helper createElement stub
test("element can be specified", function() {
view = {
isView: true,
Expand Down
2 changes: 1 addition & 1 deletion packages/ember-metal-views/tests/test_helpers.js
Expand Up @@ -46,7 +46,7 @@ MetalRenderer.prototype.createElement = function (view) {
}
}
if (view.childViews) {
view._childViewsMorph = this._dom.createMorph(el, null, null);
view._childViewsMorph = this._dom.createMorph(el, null, null, el);
} else if (view.textContent) {
el.textContent = view.textContent;
} else if (view.innerHTML) {
Expand Down
75 changes: 66 additions & 9 deletions packages/ember-views/lib/system/render_buffer.js
Expand Up @@ -6,6 +6,53 @@
import { setInnerHTML } from "ember-views/system/utils";
import jQuery from "ember-views/system/jquery";
import {DOMHelper} from "morph";
import Ember from "ember-metal/core";

// The HTML spec allows for "omitted start tags". These tags are optional
// when their intended child is the first thing in the parent tag. For
// example, this is a tbody start tag:
//
// <table>
// <tbody>
// <tr>
//
// The tbody may be omitted, and the browser will accept and render:
//
// <table>
// <tr>
//
// However, the omitted start tag will still be added to the DOM. Here
// we test the string and context to see if the browser is about to
// perform this cleanup, but with a special allowance for disregarding
// <script tags. This disregarding of <script being the first child item
// may bend the offical spec a bit, and is only needed for Handlebars
// templates.
//
// http://www.whatwg.org/specs/web-apps/current-work/multipage/syntax.html#optional-tags
// describes which tags are omittable. The spec for tbody and colgroup
// explains this behavior:
//
// http://www.whatwg.org/specs/web-apps/current-work/multipage/tables.html#the-tbody-element
// http://www.whatwg.org/specs/web-apps/current-work/multipage/tables.html#the-colgroup-element
//
var omittedStartTagChildren = {
tr: document.createElement('tbody'),
col: document.createElement('colgroup')
};

var omittedStartTagChildTest = /(?:<script)*.*?<([\w:]+)/i;

function detectOmittedStartTag(string, contextualElement){
// Omitted start tags are only inside table tags.
if (contextualElement.tagName === 'TABLE') {
var omittedStartTagChildMatch = omittedStartTagChildTest.exec(string);
if (omittedStartTagChildMatch) {
// It is already asserted that the contextual element is a table
// and not the proper start tag. Just look up the start tag.
return omittedStartTagChildren[omittedStartTagChildMatch[1].toLowerCase()];
}
}
}

function ClassSet() {
this.seen = {};
Expand Down Expand Up @@ -83,30 +130,32 @@ var canSetNameOnInputs = (function() {
to the DOM.
```javascript
var buffer = Ember.renderBuffer('div');
var buffer = Ember.renderBuffer('div', contextualElement);
```
@method renderBuffer
@namespace Ember
@param {String} tagName tag name (such as 'div' or 'p') used for the buffer
*/
export default function renderBuffer(tagName) {
return new _RenderBuffer(tagName); // jshint ignore:line
export default function renderBuffer(tagName, contextualElement) {
return new _RenderBuffer(tagName, contextualElement); // jshint ignore:line
}

function _RenderBuffer(tagName) {
function _RenderBuffer(tagName, contextualElement) {
this.tagName = tagName;
this._contextualElement = contextualElement;
this.buffer = null;
this.childViews = [];
this.dom = new DOMHelper();
}

_RenderBuffer.prototype = {

reset: function(tagName) {
reset: function(tagName, contextualElement) {
this.tagName = tagName;
this.buffer = null;
this._element = null;
this._contextualElement = contextualElement;
this.elementClasses = null;
this.elementId = null;
this.elementAttributes = null;
Expand All @@ -119,6 +168,9 @@ _RenderBuffer.prototype = {
// The root view's element
_element: null,

// The root view's contextualElement
_contextualElement: null,

/**
An internal set used to de-dupe class names when `addClass()` is
used. After each call to `addClass()`, the `classes` property
Expand Down Expand Up @@ -192,7 +244,7 @@ _RenderBuffer.prototype = {
example, if you wanted to create a `p` tag, then you would call
```javascript
Ember.RenderBuffer('p')
Ember.RenderBuffer('p', contextualElement)
```
@property elementTag
Expand Down Expand Up @@ -392,7 +444,7 @@ _RenderBuffer.prototype = {
tagString = tagName;
}

var element = document.createElement(tagString);
var element = this.dom.createElement(tagString);
var $element = jQuery(element);

if (id) {
Expand Down Expand Up @@ -446,6 +498,10 @@ _RenderBuffer.prototype = {
of this buffer
*/
element: function() {
if (!this._contextualElement) {
Ember.deprecate("buffer.element expects a contextualElement to exist. This ensures DOM that requires context is correctly generated (tr, SVG tags). Defaulting to document.body, but this will be removed in the future");
this._contextualElement = document.body;
}
var html = this.innerString();

if (this._element) {
Expand All @@ -455,10 +511,11 @@ _RenderBuffer.prototype = {
}
} else {
if (html) {
var omittedStartTag = detectOmittedStartTag(html, this._contextualElement);
var parsed = this.dom.parseHTML(html, omittedStartTag || this._contextualElement);
var frag = this._element = document.createDocumentFragment();
var parsed = jQuery.parseHTML(html);
for (var i=0,l=parsed.length; i<l; i++) {
frag.appendChild(parsed[i]);
frag.appendChild(parsed[0]); // As nodes are appended they are removed from the node list
}
this.hydrateMorphs();
} else if (html === '') {
Expand Down
4 changes: 2 additions & 2 deletions packages/ember-views/lib/system/renderer.js
Expand Up @@ -25,7 +25,7 @@ EmberRenderer.prototype.cancelRender =
};

EmberRenderer.prototype.createElement =
function EmberRenderer_createElement(view) {
function EmberRenderer_createElement(view, contextualElement) {
// If this is the top-most view, start a new buffer. Otherwise,
// create a new buffer relative to the original using the
// provided buffer operation (for example, `insertAfter` will
Expand All @@ -36,7 +36,7 @@ EmberRenderer.prototype.createElement =
}

var buffer = view.buffer = this.buffer;
buffer.reset(tagName);
buffer.reset(tagName, contextualElement);

if (view.beforeRender) {
view.beforeRender(buffer);
Expand Down
4 changes: 2 additions & 2 deletions packages/ember-views/lib/views/container_view.js
Expand Up @@ -269,10 +269,10 @@ var ContainerView = View.extend(MutableArray, {
this._childViewsMorph = this._morph;
} else {
element = dom.createDocumentFragment();
this._childViewsMorph = dom.appendMorph(element);
this._childViewsMorph = dom.appendMorph(element, element);
}
} else {
this._childViewsMorph = dom.createMorph(element, element.lastChild, null);
this._childViewsMorph = dom.createMorph(element, element.lastChild, null, element);
}

return element;
Expand Down
1 change: 1 addition & 0 deletions packages/ember-views/lib/views/view.js
Expand Up @@ -1439,6 +1439,7 @@ var View = CoreView.extend({
createElement: function() {
if (this.element) { return this; }

this._didCreateElementWithoutMorph = true;
this.constructor.renderer.renderTree(this);

return this;
Expand Down

0 comments on commit 9cb03e8

Please sign in to comment.