From 6a95af596851d69dfe5220c3c074d12b3083d87f Mon Sep 17 00:00:00 2001 From: Matthew Beale Date: Tue, 27 Oct 2015 12:15:29 -0400 Subject: [PATCH] Parse nbsp into spaces, render nbsp where needed * Mobiledoc should, ideally, not contain any HTML entities. Instead these should be converted into their property plain text pair. This changes the parsing code to disallow nbsp chars, instead converting them into regular spaces. * The Content-Kit renderer must display spaces correctly when there are multiple spaces in a row. This patch ensures that at least every other space character is displayed as an nbsp. Fixes #195 --- src/js/parsers/dom.js | 10 +++++- src/js/parsers/section.js | 10 ++++-- src/js/renderers/editor-dom.js | 33 +++++++++++------- tests/acceptance/editor-selections-test.js | 27 +++++++++++++++ tests/unit/parsers/dom-test.js | 39 ++++++++++++++++++++-- tests/unit/renderers/editor-dom-test.js | 15 +++++++++ 6 files changed, 116 insertions(+), 18 deletions(-) diff --git a/src/js/parsers/dom.js b/src/js/parsers/dom.js index 221ee7f7c..34c539870 100644 --- a/src/js/parsers/dom.js +++ b/src/js/parsers/dom.js @@ -1,3 +1,4 @@ +import { NO_BREAK_SPACE } from '../renderers/editor-dom'; import { MARKUP_SECTION_TYPE, LIST_SECTION_TYPE, @@ -18,6 +19,13 @@ import Markup from 'content-kit-editor/models/markup'; const GOOGLE_DOCS_CONTAINER_ID_REGEX = /^docs\-internal\-guid/; +const NO_BREAK_SPACE_REGEX = new RegExp(NO_BREAK_SPACE, 'g'); +export function transformHTMLText(textContent) { + let text = textContent; + text = text.replace(NO_BREAK_SPACE_REGEX, ' '); + return text; +} + function isGoogleDocsContainer(element) { return !isTextNode(element) && normalizeTagName(element.tagName) === normalizeTagName('b') && @@ -154,7 +162,7 @@ export default class DOMParser { let previousMarker; walkTextNodes(element, (textNode) => { - const text = textNode.textContent; + const text = transformHTMLText(textNode.textContent); let markups = this.collectMarkups(textNode, element); let marker; diff --git a/src/js/parsers/section.js b/src/js/parsers/section.js index 72c496d50..48f5cbde4 100644 --- a/src/js/parsers/section.js +++ b/src/js/parsers/section.js @@ -35,6 +35,10 @@ import { contains } from 'content-kit-editor/utils/array-utils'; +import { + transformHTMLText +} from '../parsers/dom'; + function isListSection(section) { return section.type === LIST_SECTION_TYPE; } @@ -164,8 +168,7 @@ export default class SectionParser { // close a trailing text node if it exists if (state.text.length) { - let marker = this.builder.createMarker(state.text, state.markups); - state.section.markers.append(marker); + this._createMarker(); } sections.push(state.section); @@ -222,7 +225,8 @@ export default class SectionParser { _createMarker() { let { state } = this; - let marker = this.builder.createMarker(state.text, state.markups); + let text = transformHTMLText(state.text); + let marker = this.builder.createMarker(text, state.markups); state.section.markers.append(marker); state.text = ''; } diff --git a/src/js/renderers/editor-dom.js b/src/js/renderers/editor-dom.js index b1852e8e3..3d2ed2783 100644 --- a/src/js/renderers/editor-dom.js +++ b/src/js/renderers/editor-dom.js @@ -24,6 +24,26 @@ function createElementFromMarkup(doc, markup) { return element; } +// FIXME: This can be done more efficiently with a single pass +// building a correct string based on the original. +function renderHTMLText(marker) { + let text = marker.value; + // If the first marker has a leading space or the last marker has a + // trailing space, the browser will collapse the space when we position + // the cursor. + // See https://github.com/bustlelabs/content-kit-editor/issues/68 + // and https://github.com/bustlelabs/content-kit-editor/issues/75 + if (!marker.next && endsWith(text, SPACE)) { + text = text.substr(0, text.length - 1) + NO_BREAK_SPACE; + } else if ((!marker.prev || endsWith(marker.prev.value, SPACE)) && startsWith(text, SPACE)) { + text = NO_BREAK_SPACE + text.substr(1); + } + text = text.replace(/ ( )/g, () => { + return ' '+NO_BREAK_SPACE; + }); + return text; +} + // ascends from element upward, returning the last parent node that is not // parentElement function penultimateParentOf(element, parentElement) { @@ -79,18 +99,7 @@ function getNextMarkerElement(renderNode) { } function renderMarker(marker, element, previousRenderNode) { - let text = marker.value; - - // If the first marker has a leading space or the last marker has a - // trailing space, the browser will collapse the space when we position - // the cursor. - // See https://github.com/bustlelabs/content-kit-editor/issues/68 - // and https://github.com/bustlelabs/content-kit-editor/issues/75 - if (!marker.next && endsWith(text, SPACE)) { - text = text.substr(0, text.length - 1) + NO_BREAK_SPACE; - } else if (!marker.prev && startsWith(text, SPACE)) { - text = NO_BREAK_SPACE + text.substr(1); - } + let text = renderHTMLText(marker); let textNode = document.createTextNode(text); let currentElement = textNode; diff --git a/tests/acceptance/editor-selections-test.js b/tests/acceptance/editor-selections-test.js index 2b3beb327..4a0a90585 100644 --- a/tests/acceptance/editor-selections-test.js +++ b/tests/acceptance/editor-selections-test.js @@ -8,6 +8,18 @@ const { test, module } = Helpers; let fixture, editor, editorElement; +const mobileDocWithSection = { + version: MOBILEDOC_VERSION, + sections: [ + [], + [ + [1, "P", [ + [[], 0, "one trick pony"] + ]] + ] + ] +}; + const mobileDocWith2Sections = { version: MOBILEDOC_VERSION, sections: [ @@ -252,6 +264,21 @@ test('keystroke of printable character while text is selected deletes the text', 'updates the section'); }); +test('selecting text bounded by space and typing replaces it', (assert) => { + editor = new Editor({mobiledoc: mobileDocWithSection}); + editor.render(editorElement); + + Helpers.dom.selectText('trick', editorElement); + Helpers.dom.insertText(editor, 'X'); + + assert.hasElement('#editor p:contains(one X pony)', + 'new text present'); + + Helpers.dom.insertText(editor, 'Y'); + assert.hasElement('#editor p:contains(one XY pony)', + 'cursor positioned correctly'); +}); + test('selecting all text across sections and hitting enter deletes and moves cursor to empty section', (assert) => { editor = new Editor({mobiledoc: mobileDocWith2Sections}); editor.render(editorElement); diff --git a/tests/unit/parsers/dom-test.js b/tests/unit/parsers/dom-test.js index 8b24b2f91..b781e6d66 100644 --- a/tests/unit/parsers/dom-test.js +++ b/tests/unit/parsers/dom-test.js @@ -2,6 +2,7 @@ import DOMParser from 'content-kit-editor/parsers/dom'; import PostNodeBuilder from 'content-kit-editor/models/post-node-builder'; import Helpers from '../../test-helpers'; import { Editor } from 'content-kit-editor'; +import { NO_BREAK_SPACE } from 'content-kit-editor/renderers/editor-dom'; const {module, test} = Helpers; @@ -50,8 +51,17 @@ test('#parse can parse multiple elements', (assert) => { assert.equal(s2.markers.head.value, 'some other text'); }); +test('#parse can parse spaces and breaking spaces', (assert) => { + let element = buildDOM("

some  text   for    you

"); + + const post = parser.parse(element); + const s1 = post.sections.head; + assert.equal(s1.markers.length, 1, 's1 has 1 marker'); + assert.equal(s1.markers.head.value, 'some text for you', 'has text'); +}); + test('editor#reparse catches changes to section', (assert) => { - const mobiledoc = Helpers.mobiledoc.build(({post, markupSection, marker}) => + const mobiledoc = Helpers.mobiledoc.build(({post, markupSection, marker}) => post([ markupSection('p', [marker('the marker')]) ]) @@ -75,8 +85,33 @@ test('editor#reparse catches changes to section', (assert) => { assert.equal(section.text, 'the NEW marker'); }); +test('editor#reparse parses spaces and breaking spaces', (assert) => { + const mobiledoc = Helpers.mobiledoc.build(({post, markupSection, marker}) => + post([ + markupSection('p', [marker('the marker')]) + ]) + ); + editor = new Editor({mobiledoc}); + const editorElement = $('
')[0]; + $('#qunit-fixture').append(editorElement); + editor.render(editorElement); + + assert.hasElement('#editor p:contains(the marker)', 'precond - rendered correctly'); + + const p = $('#editor p:eq(0)')[0]; + p.childNodes[0].textContent = `some ${NO_BREAK_SPACE}text ${NO_BREAK_SPACE}${NO_BREAK_SPACE}for ${NO_BREAK_SPACE} ${NO_BREAK_SPACE}you`; + + // In Firefox, changing the text content changes the selection, so re-set it + Helpers.dom.moveCursorTo(p.childNodes[0]); + + editor.reparse(); + + const section = editor.post.sections.head; + assert.equal(section.text, 'some text for you'); +}); + test('editor#reparse catches changes to list section', (assert) => { - const mobiledoc = Helpers.mobiledoc.build(({post, listSection, listItem, marker}) => + const mobiledoc = Helpers.mobiledoc.build(({post, listSection, listItem, marker}) => post([ listSection('ul', [ listItem([marker('the list item')]) diff --git a/tests/unit/renderers/editor-dom-test.js b/tests/unit/renderers/editor-dom-test.js index 2c247be1c..b016a1121 100644 --- a/tests/unit/renderers/editor-dom-test.js +++ b/tests/unit/renderers/editor-dom-test.js @@ -2,6 +2,7 @@ import PostNodeBuilder from 'content-kit-editor/models/post-node-builder'; import Renderer from 'content-kit-editor/renderers/editor-dom'; import RenderTree from 'content-kit-editor/models/render-tree'; import Helpers from '../../test-helpers'; +import { NO_BREAK_SPACE } from 'content-kit-editor/renderers/editor-dom'; const { module, test } = Helpers; const DATA_URL = "data:image/gif;base64,R0lGODlhAQABAIAAAP///wAAACwAAAAAAQABAAACAkQBADs="; @@ -589,6 +590,20 @@ test('renders markup section "pull-quote" as
', (a assert.equal(renderTree.rootElement.innerHTML, expectedDOM.outerHTML); }); +test('renders a bunch of spaces with nbsp', (assert) => { + const post = Helpers.postAbstract.build(({post, markupSection, marker}) => { + return post([markupSection('p', [marker('a b c d ')])]); + }); + const renderTree = new RenderTree(post); + render(renderTree); + + const expectedDOM = Helpers.dom.build(t => { + return t('p', {}, [t.text(`a b ${NO_BREAK_SPACE}c ${NO_BREAK_SPACE} ${NO_BREAK_SPACE}d${NO_BREAK_SPACE}`)]); + }); + + assert.equal(renderTree.rootElement.innerHTML, expectedDOM.outerHTML); +}); + /* test("It renders a renderTree with rendered dirty section", (assert) => { /*