Skip to content

Commit

Permalink
[fieldFormat] make formatted output non-bindable (#11911)
Browse files Browse the repository at this point in the history
* [fieldFormat] make formatted output non-bindable

* [aggTable] totals are consumed as text, produce text

* [tests] fix existing tests

* [fieldFormat/contentType] add tests for non-bindable wrapper

(cherry picked from commit 1d2341f)
  • Loading branch information
spalger committed May 19, 2017
1 parent 9751d33 commit f85570d
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 27 deletions.
4 changes: 2 additions & 2 deletions src/ui/public/agg_table/agg_table.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ uiModules
.get('kibana')
.directive('kbnAggTable', function ($filter, config, Private, compileRecursiveDirective) {
const fieldFormats = Private(RegistryFieldFormatsProvider);
const numberFormatter = fieldFormats.getDefaultInstance('number').getConverterFor('html');
const numberFormatter = fieldFormats.getDefaultInstance('number').getConverterFor('text');

return {
restrict: 'E',
Expand Down Expand Up @@ -130,7 +130,7 @@ uiModules
return prev + curr[i].value;
}, 0);
}
const formatter = agg.fieldFormatter('html');
const formatter = agg.fieldFormatter('text');

switch ($scope.totalFunc) {
case 'sum':
Expand Down
12 changes: 7 additions & 5 deletions src/ui/public/index_patterns/__tests__/_field_format.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import _ from 'lodash';
import expect from 'expect.js';
import ngMock from 'ng_mock';
import IndexPatternsFieldFormatFieldFormatProvider from 'ui/index_patterns/_field_format/field_format';

describe('FieldFormat class', function () {

let FieldFormat;
Expand Down Expand Up @@ -86,7 +87,7 @@ describe('FieldFormat class', function () {
const html = f.getConverterFor('html');
expect(text).to.not.be(html);
expect(text('formatted')).to.be('formatted');
expect(html('formatted')).to.be('formatted');
expect(html('formatted')).to.be('<span ng-non-bindable>formatted</span>');
});

it('can be an object, with seperate text and html converter', function () {
Expand All @@ -100,7 +101,7 @@ describe('FieldFormat class', function () {
const html = f.getConverterFor('html');
expect(text).to.not.be(html);
expect(text('formatted text')).to.be('formatted text');
expect(html('formatted html')).to.be('formatted html');
expect(html('formatted html')).to.be('<span ng-non-bindable>formatted html</span>');
});

it('does not escape the output of the text converter', function () {
Expand All @@ -112,7 +113,8 @@ describe('FieldFormat class', function () {
it('does escape the output of the text converter if used in an html context', function () {
TestFormat.prototype._convert = _.constant('<script>alert("xxs");</script>');
const f = new TestFormat();
expect(f.convert('', 'html')).to.not.contain('<');
expect(_.trimRight(_.trimLeft(f.convert('', 'html'), '<span ng-non-bindable>'), '</span>'))
.to.not.contain('<');
});

it('does not escape the output of an html specific converter', function () {
Expand All @@ -123,7 +125,7 @@ describe('FieldFormat class', function () {

const f = new TestFormat();
expect(f.convert('', 'text')).to.be('<img>');
expect(f.convert('', 'html')).to.be('<img>');
expect(f.convert('', 'html')).to.be('<span ng-non-bindable><img></span>');
});
});

Expand All @@ -145,7 +147,7 @@ describe('FieldFormat class', function () {
};

const f = new TestFormat();
expect(f.convert('val', 'html')).to.be('html');
expect(f.convert('val', 'html')).to.be('<span ng-non-bindable>html</span>');
});

it('formats a value as " - " when no value is specified', function () {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import angular from 'angular';
import $ from 'jquery';
import ngMock from 'ng_mock';
import sinon from 'sinon';
import expect from 'expect.js';
import { escape } from 'lodash';

import { IndexPatternsFieldFormatContentTypesProvider } from '../content_types';

describe('index_patterns/_field_format/content_types', () => {

let render;
const callMe = sinon.stub();
afterEach(() => callMe.reset());

function getAllContents(node) {
return [...node.childNodes].reduce((acc, child) => {
return acc.concat(child, getAllContents(child));
}, []);
}

angular.module('testApp', [])
.directive('testDirective', () => ({
restrict: 'EACM',
link: callMe
}));

beforeEach(ngMock.module('testApp'));
beforeEach(ngMock.inject(($injector) => {
const contentTypes = new IndexPatternsFieldFormatContentTypesProvider();
const $rootScope = $injector.get('$rootScope');
const $compile = $injector.get('$compile');

$rootScope.callMe = callMe;

render = (convert) => {
const $el = $('<div>');
const { html } = contentTypes.setup({ _convert: { html: convert } });
$compile($el.html(html(`
<!-- directive: test-directive -->
<div></div>
<test-directive>{{callMe()}}</test-directive>
<span test-directive></span>
<marquee class="test-directive"></marquee>
`)))($rootScope);
return $el;
};
}));

it('no element directive', () => {
const $el = render(value => `
<test-directive>${escape(value)}</test-directive>
`);

expect($el.find('test-directive')).to.have.length(1);
sinon.assert.notCalled(callMe);
});

it('no attribute directive', () => {
const $el = render(value => `
<div test-directive>${escape(value)}</div>
`);

expect($el.find('[test-directive]')).to.have.length(1);
sinon.assert.notCalled(callMe);
});

it('no comment directive', () => {
const $el = render(value => `
<!-- directive: test-directive -->
<div>${escape(value)}</div>
`);

const comments = getAllContents($el.get(0))
.filter(node => node.nodeType === 8);

expect(comments).to.have.length(1);
expect(comments[0].textContent).to.contain('test-directive');
sinon.assert.notCalled(callMe);
});

it('no class directive', () => {
const $el = render(value => `
<div class="test-directive">${escape(value)}</div>
`);

expect($el.find('.test-directive')).to.have.length(1);
sinon.assert.notCalled(callMe);
});

it('no interpolation', () => {
const $el = render(value => `
<div class="foo {{callMe()}}">${escape(value)}</div>
`);

expect($el.find('.foo')).to.have.length(1);
sinon.assert.notCalled(callMe);
});
});
6 changes: 5 additions & 1 deletion src/ui/public/index_patterns/_field_format/content_types.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export default function contentTypesProvider() {

const types = {
html: function (format, convert) {
return function recurse(value, field, hit) {
function recurse(value, field, hit) {
if (value == null) {
return _.asPrettyString(value);
}
Expand All @@ -22,6 +22,10 @@ export default function contentTypesProvider() {
});

return subVals.join(',' + (useMultiLine ? '\n' : ' '));
}

return function (...args) {
return `<span ng-non-bindable>${recurse(...args)}</span>`;
};
},

Expand Down
38 changes: 25 additions & 13 deletions src/ui/public/stringify/__tests__/_color.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,14 @@ describe('Color Format', function () {
background: 'yellow'
}]
});
expect(colorer.convert(99, 'html')).to.eql('99');
expect(colorer.convert(100, 'html')).to.eql('<span style="color: blue;background-color: yellow;">100</span>');
expect(colorer.convert(150, 'html')).to.eql('<span style="color: blue;background-color: yellow;">150</span>');
expect(colorer.convert(151, 'html')).to.eql('151');
expect(colorer.convert(99, 'html')).to.eql('<span ng-non-bindable>99</span>');
expect(colorer.convert(100, 'html')).to.eql(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">100</span></span>'
);
expect(colorer.convert(150, 'html')).to.eql(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">150</span></span>'
);
expect(colorer.convert(151, 'html')).to.eql('<span ng-non-bindable>151</span>');
});

it('should not convert invalid ranges', function () {
Expand All @@ -36,7 +40,7 @@ describe('Color Format', function () {
background: 'yellow'
}]
});
expect(colorer.convert(99, 'html')).to.eql('99');
expect(colorer.convert(99, 'html')).to.eql('<span ng-non-bindable>99</span>');
});
});

Expand All @@ -52,15 +56,23 @@ describe('Color Format', function () {
});

const converter = colorer.getConverterFor('html');
expect(converter('B', 'html')).to.eql('B');
expect(converter('AAA', 'html')).to.eql('<span style="color: blue;background-color: yellow;">AAA</span>');
expect(converter('AB', 'html')).to.eql('<span style="color: blue;background-color: yellow;">AB</span>');
expect(converter('a', 'html')).to.eql('a');
expect(converter('B', 'html')).to.eql('<span ng-non-bindable>B</span>');
expect(converter('AAA', 'html')).to.eql(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">AAA</span></span>'
);
expect(converter('AB', 'html')).to.eql(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">AB</span></span>'
);
expect(converter('a', 'html')).to.eql('<span ng-non-bindable>a</span>');

expect(converter('B', 'html')).to.eql('B');
expect(converter('AAA', 'html')).to.eql('<span style="color: blue;background-color: yellow;">AAA</span>');
expect(converter('AB', 'html')).to.eql('<span style="color: blue;background-color: yellow;">AB</span>');
expect(converter('a', 'html')).to.eql('a');
expect(converter('B', 'html')).to.eql('<span ng-non-bindable>B</span>');
expect(converter('AAA', 'html')).to.eql(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">AAA</span></span>'
);
expect(converter('AB', 'html')).to.eql(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">AB</span></span>'
);
expect(converter('a', 'html')).to.eql('<span ng-non-bindable>a</span>');
});
});
});
6 changes: 4 additions & 2 deletions src/ui/public/stringify/__tests__/_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@ describe('_source formatting', function () {

it('should use the text content type if a field is not passed', function () {
const hit = _.first(hits);
expect(convertHtml(hit._source)).to.be(JSON.stringify(hit._source));
expect(convertHtml(hit._source)).to.be(`<span ng-non-bindable>${JSON.stringify(hit._source)}</span>`);
});

it('uses the _source, field, and hit to create a <dl>', function () {
const hit = _.first(hits);
const $dl = $(convertHtml(hit._source, indexPattern.fields.byName._source, hit));
const $nonBindable = $(convertHtml(hit._source, indexPattern.fields.byName._source, hit));
expect($nonBindable.is('span[ng-non-bindable]')).to.be.ok();
const $dl = $nonBindable.children();
expect($dl.is('dl')).to.be.ok();
expect($dl.find('dt')).to.have.length(_.keys(indexPattern.flattenHit(hit)).length);
});
Expand Down
13 changes: 9 additions & 4 deletions src/ui/public/stringify/__tests__/_url.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ describe('Url Format', function () {
fieldFormats = Private(RegistryFieldFormatsProvider);
}));

const unwrap = $el => {
expect($el.is('span[ng-non-bindable]')).to.be.ok();
return $el.children();
};

describe('Url Format', function () {
let Url;

Expand All @@ -21,7 +26,7 @@ describe('Url Format', function () {
it('ouputs a simple <a> tab by default', function () {
const url = new Url();

const $a = $(url.convert('http://elastic.co', 'html'));
const $a = unwrap($(url.convert('http://elastic.co', 'html')));
expect($a.is('a')).to.be(true);
expect($a.size()).to.be(1);
expect($a.attr('href')).to.be('http://elastic.co');
Expand All @@ -32,15 +37,15 @@ describe('Url Format', function () {
it('outputs an <image> if type === "img"', function () {
const url = new Url({ type: 'img' });

const $img = $(url.convert('http://elastic.co', 'html'));
const $img = unwrap($(url.convert('http://elastic.co', 'html')));
expect($img.is('img')).to.be(true);
expect($img.attr('src')).to.be('http://elastic.co');
});

describe('url template', function () {
it('accepts a template', function () {
const url = new Url({ urlTemplate: 'url: {{ value }}' });
const $a = $(url.convert('url', 'html'));
const $a = unwrap($(url.convert('url', 'html')));
expect($a.is('a')).to.be(true);
expect($a.size()).to.be(1);
expect($a.attr('href')).to.be('url: url');
Expand All @@ -57,7 +62,7 @@ describe('Url Format', function () {
describe('label template', function () {
it('accepts a template', function () {
const url = new Url({ labelTemplate: 'extension: {{ value }}' });
const $a = $(url.convert('php', 'html'));
const $a = unwrap($(url.convert('php', 'html')));
expect($a.is('a')).to.be(true);
expect($a.size()).to.be(1);
expect($a.attr('href')).to.be('php');
Expand Down

0 comments on commit f85570d

Please sign in to comment.