From ca88c781312aa2d2a9ea5e6044e32e928e2e3e66 Mon Sep 17 00:00:00 2001 From: Joe Covalesky Date: Mon, 16 Dec 2013 15:50:20 -0800 Subject: [PATCH 1/5] fix FF contenteditable backspace bug. https://github.com/yui/yui3/issues/1376 --- src/editor/js/editor-para.js | 53 +++++-- .../unit/assets/editor-content-editable.js | 149 ++++++++++++++---- 2 files changed, 157 insertions(+), 45 deletions(-) diff --git a/src/editor/js/editor-para.js b/src/editor/js/editor-para.js index 1bcf142f2e8..7e2567e20f7 100644 --- a/src/editor/js/editor-para.js +++ b/src/editor/js/editor-para.js @@ -247,17 +247,9 @@ } } } + if (Y.UA.gecko) { - /* - * This forced FF to redraw the content on backspace. - * On some occasions FF will leave a cursor residue after content has been deleted. - * Dropping in the empty textnode and then removing it causes FF to redraw and - * remove the "ghost cursors" - */ - d = e.changedNode; - t = inst.config.doc.createTextNode(' '); - d.appendChild(t); - d.removeChild(t); + this._fixGeckoOnBackspace(inst); } break; } @@ -271,6 +263,46 @@ } }, + + //If we just backspaced into a P on FF, we have to put the cursor + //before the BR that FF (usually) had injected when we used to + //leave the P. + _fixGeckoOnBackspace: function (inst) { + var sel = new inst.EditorSelection(), + offset, + node, + childNodes; + + //not a cursor, not in a paragraph, or anchored at paragraph start. + if (!sel.isCollapsed || !sel.anchorNode.get('tagName') === 'P' || + sel.anchorOffset === 0) { + return; + } + + //cursor not on the injected final BR + childNodes = sel.anchorNode.get('childNodes'); + node = sel.anchorNode.get('lastChild'); + if (sel.anchorOffset !== childNodes.size() || node.get('tagName') !== 'BR') { + return; + } + + //empty P (only contains BR) + if (sel.anchorOffset === 1) { + sel.selectNode(sel.anchorNode, true); + return; + } + + //We only expect injected BR behavior when last Node is text + node = node.get('previousSibling'); + if (node.get('nodeType') !== Node.TEXT_NODE) { + return; + } + + offset = node.get('length'); + sel.selectNode(node, true, offset); + + }, + initializer: function() { var host = this.get(HOST); if (host.editorBR) { @@ -303,4 +335,3 @@ Y.namespace('Plugin'); Y.Plugin.EditorPara = EditorPara; - diff --git a/src/editor/tests/unit/assets/editor-content-editable.js b/src/editor/tests/unit/assets/editor-content-editable.js index b29a049ccb6..02a25826aed 100644 --- a/src/editor/tests/unit/assets/editor-content-editable.js +++ b/src/editor/tests/unit/assets/editor-content-editable.js @@ -24,9 +24,14 @@ YUI.add('editor-tests', function(Y) { template = { name: 'Inline Editor Tests', - setUp : function() {}, - tearDown : function() {}, + setUp : function() { + this.editorBaseUse = Y.EditorBase.USE.concat('node-event-simulate', + 'dd'); + }, + + tearDown : function() { + }, 'test_load ': function() { Y.Assert.isObject(Y.Plugin.ContentEditable, 'ContentEditable was not loaded'); @@ -347,8 +352,6 @@ YUI.add('editor-tests', function(Y) { Y.Assert.isFalse(Y.one('#editor').hasAttribute('contenteditable'), 'iframe DOM node was not destroyed'); }, 'test_editor ': function() { - Y.EditorBase.USE.push('dd'); - Y.EditorBase.USE.push('node-event-simulate'); var iframeReady = false; editor = new Y.EditorBase({ @@ -358,7 +361,7 @@ YUI.add('editor-tests', function(Y) { { fn: Y.Plugin.ContentEditable, cfg: { - use: Y.EditorBase.USE + use: this.editorBaseUse } } ] @@ -613,7 +616,7 @@ YUI.add('editor-tests', function(Y) { { fn: Y.Plugin.ContentEditable, cfg: { - use: Y.EditorBase.USE + use: this.editorBaseUse } } ] @@ -631,52 +634,130 @@ YUI.add('editor-tests', function(Y) { Y.Assert.areEqual(Y.one('#editor iframe'), null, 'Second Frame was not destroyed'); Y.Assert.isFalse(Y.one('#editor').hasAttribute('contenteditable'), 'iframe DOM node was not destroyed'); }, - 'test_para_plugin ': function() { + + makeEditorWithParaPlugin : function (initialContent, onReady) { + var _this = this, + needResume = false; + editor = new Y.EditorBase({ - content: 'Hello World!!', + content: initialContent, extracss: 'b { color: red; }', plugins: [ { fn: Y.Plugin.ContentEditable, cfg: { - use: Y.EditorBase.USE + use: this.editorBaseUse } } ] }); - Y.Assert.isInstanceOf(Y.EditorBase, editor, 'Third EditorBase instance can not be created'); + + Y.Assert.isInstanceOf(Y.EditorBase, editor, 'EditorBase instance not created?'); editor.plug(Y.Plugin.EditorPara); - Y.Assert.isInstanceOf(Y.Plugin.EditorPara, editor.editorPara, 'EditorPara was not plugged..'); + Y.Assert.isInstanceOf(Y.Plugin.EditorPara, editor.editorPara, 'EditorPara was not plugged.'); + + if (onReady) { + editor.after('ready', function () { + if (needResume) { + _this.resume(onReady); + } else { + onReady(); + } + }); + } + editor.render('#editor'); - editor.set('content', '
Test This'); + needResume = true; + }, - var inst = editor.getInstance(); + 'test_para_plugin ': function() { + var inst, str, out, editorReady; - var str = 'foo'; - var out = editor.frame.exec._wrapContent(str); - Y.Assert.areEqual('

foo

', out); + function onEditorReady() { + editorReady = true; - out = editor.frame.exec._wrapContent(str, true); - Y.Assert.areEqual('foo
', out); + editor.set('content', '
Test This'); - fireKey(editor, 13); - fireKey(editor, 8); - editor.editorPara._fixFirstPara(); - editor.editorPara._afterPaste(); - editor.editorPara._onNodeChange({ - changedEvent: {}, - changedNode: inst.one('b'), - changedType: 'enter-up' - }); - editor.editorPara._onNodeChange({ - changedEvent: {}, - changedNode: inst.one('br'), - changedType: 'enter' - }); - editor.destroy(); - Y.Assert.areEqual(Y.one('#editor iframe'), null, 'Third Frame was not destroyed'); + inst = editor.getInstance(); + + str = 'foo'; + out = editor.frame.exec._wrapContent(str); + Y.Assert.areEqual('

foo

', out); + out = editor.frame.exec._wrapContent(str, true); + Y.Assert.areEqual('foo
', out); + + fireKey(editor, 13); + fireKey(editor, 8); + editor.editorPara._fixFirstPara(); + editor.editorPara._afterPaste(); + editor.editorPara._onNodeChange({ + changedEvent: {}, + changedNode: inst.one('b'), + changedType: 'enter-up' + }); + editor.editorPara._onNodeChange({ + changedEvent: {}, + changedNode: inst.one('br'), + changedType: 'enter' + }); + + editor.detachAll(); + editor.destroy(); + Y.Assert.areEqual(Y.one('#editor iframe'), null, 'Third Frame was not destroyed'); + } + + this.makeEditorWithParaPlugin('

content

', onEditorReady); + + //will fail if onEditorReady not called within 3s + if (!editorReady) { + this.wait(3000); + } }, + + 'test para plugin gecko fix ' : function () { + var editorReady = false; + + function onEditorReady() { + var inst = editor.frame.getInstance(), + container = editor.frame.get('container'), + node = container.getDOMNode(), + sel = new inst.EditorSelection(), + text = 'here is the initial text'; + + //Simulate gecko state where: P is terminated by BR, then we + //backspace into it and the cursor (collapsed selection) is + //after the BR. See: https://github.com/yui/yui3/issues/1376 + editor.set('content', '

' + text + '

'); + container.one('p').appendChild('
'); + //place cursor 2 nodes after P start (1st is text node, 2nd is BR) + node = container.one('p'); + sel.selectNode(node, true, 2); + + editor.editorPara._fixGeckoOnBackspace(inst); + + //Cursor should now be before the BR + sel = new inst.EditorSelection(); + Y.Assert.isTrue(sel.isCollapsed, 'expected cursor, not range for selection'); + Y.Assert.areEqual(1, container.all('p').size()); + node = container.one('p'); + Y.Assert.areEqual('P', sel.anchorNode.get('tagName'), + 'selection anchorNode wrong'); + Y.Assert.areEqual('P', sel.focusNode.get('tagName'), + 'selection focusNode wrong'); + Y.Assert.areEqual(text.length, sel.anchorOffset, + 'selection anchorOffset wrong'); + editorReady = true; + } + + this.makeEditorWithParaPlugin('

content

', onEditorReady); + + //If editor isn't ready within 3s, fail. + if (!editorReady) { + this.wait(3000); + } + }, + 'test_double_plug_setup ': function() { editor = new Y.EditorBase({ content: 'Hello World!!', From f1a1c7fdf83d714ebd7bd2fb993820f182b87444 Mon Sep 17 00:00:00 2001 From: Joe Covalesky Date: Thu, 9 Jan 2014 17:22:32 -0800 Subject: [PATCH 2/5] tweaks per YUI team advice on pull request. https://github.com/yui/yui3/pull/1502 --- src/editor/tests/unit/assets/editor-content-editable.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/editor/tests/unit/assets/editor-content-editable.js b/src/editor/tests/unit/assets/editor-content-editable.js index 02a25826aed..0cef792e837 100644 --- a/src/editor/tests/unit/assets/editor-content-editable.js +++ b/src/editor/tests/unit/assets/editor-content-editable.js @@ -741,9 +741,9 @@ YUI.add('editor-tests', function(Y) { Y.Assert.isTrue(sel.isCollapsed, 'expected cursor, not range for selection'); Y.Assert.areEqual(1, container.all('p').size()); node = container.one('p'); - Y.Assert.areEqual('P', sel.anchorNode.get('tagName'), + Y.Assert.areEqual('P', sel.anchorNode.get('nodeName'), 'selection anchorNode wrong'); - Y.Assert.areEqual('P', sel.focusNode.get('tagName'), + Y.Assert.areEqual('P', sel.focusNode.get('nodeName'), 'selection focusNode wrong'); Y.Assert.areEqual(text.length, sel.anchorOffset, 'selection anchorOffset wrong'); @@ -995,7 +995,8 @@ YUI.add('editor-tests', function(Y) { ignore: { 'test: EditorSelection ': Y.UA.phantomjs, 'test_selection_methods ': Y.UA.phantomjs, - 'test_br_plugin ': Y.UA.phantomjs + 'test_br_plugin ': Y.UA.phantomjs, + 'test para plugin gecko fix ': !Y.UA.gecko }, error: { //These tests should error 'test_selection_methods ': (Y.UA.ie ? true : false), From a3f1dea5d00b6351d71b32e09c84a46889a29840 Mon Sep 17 00:00:00 2001 From: Joe Covalesky Date: Thu, 9 Jan 2014 17:24:45 -0800 Subject: [PATCH 3/5] tweaks per YUI team advice on pull request. https://github.com/yui/yui3/pull/1502 --- src/editor/js/editor-para.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/editor/js/editor-para.js b/src/editor/js/editor-para.js index 7e2567e20f7..bb456318a5a 100644 --- a/src/editor/js/editor-para.js +++ b/src/editor/js/editor-para.js @@ -269,12 +269,11 @@ //leave the P. _fixGeckoOnBackspace: function (inst) { var sel = new inst.EditorSelection(), - offset, node, childNodes; //not a cursor, not in a paragraph, or anchored at paragraph start. - if (!sel.isCollapsed || !sel.anchorNode.get('tagName') === 'P' || + if (!sel.isCollapsed || sel.anchorNode.get('nodeName') !== 'P' || sel.anchorOffset === 0) { return; } @@ -282,7 +281,7 @@ //cursor not on the injected final BR childNodes = sel.anchorNode.get('childNodes'); node = sel.anchorNode.get('lastChild'); - if (sel.anchorOffset !== childNodes.size() || node.get('tagName') !== 'BR') { + if (sel.anchorOffset !== childNodes.size() || node.get('nodeName') !== 'BR') { return; } @@ -294,13 +293,9 @@ //We only expect injected BR behavior when last Node is text node = node.get('previousSibling'); - if (node.get('nodeType') !== Node.TEXT_NODE) { - return; + if (node.get('nodeType') === Node.TEXT_NODE) { + sel.selectNode(node, true, node.get('length')); } - - offset = node.get('length'); - sel.selectNode(node, true, offset); - }, initializer: function() { From f8f8126f3745e4ec73117b4fc5a1dc747753b18f Mon Sep 17 00:00:00 2001 From: Joe Covalesky Date: Wed, 5 Feb 2014 21:58:05 -0800 Subject: [PATCH 4/5] tweaks per https://github.com/yui/yui3/pull/1502/files#r8857828 --- .../unit/assets/editor-content-editable.js | 32 +++++-------------- 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/src/editor/tests/unit/assets/editor-content-editable.js b/src/editor/tests/unit/assets/editor-content-editable.js index 0cef792e837..9dd0077988b 100644 --- a/src/editor/tests/unit/assets/editor-content-editable.js +++ b/src/editor/tests/unit/assets/editor-content-editable.js @@ -636,8 +636,7 @@ YUI.add('editor-tests', function(Y) { }, makeEditorWithParaPlugin : function (initialContent, onReady) { - var _this = this, - needResume = false; + var _this = this; editor = new Y.EditorBase({ content: initialContent, @@ -658,24 +657,20 @@ YUI.add('editor-tests', function(Y) { if (onReady) { editor.after('ready', function () { - if (needResume) { _this.resume(onReady); - } else { - onReady(); - } }); } - editor.render('#editor'); - needResume = true; + //force to always be async onReady call + setTimeout( function () { + editor.render('#editor'); + }, 0); }, 'test_para_plugin ': function() { - var inst, str, out, editorReady; + var inst, str, out; function onEditorReady() { - editorReady = true; - editor.set('content', '
Test This'); inst = editor.getInstance(); @@ -708,16 +703,10 @@ YUI.add('editor-tests', function(Y) { } this.makeEditorWithParaPlugin('

content

', onEditorReady); - - //will fail if onEditorReady not called within 3s - if (!editorReady) { - this.wait(3000); - } + this.wait(); }, 'test para plugin gecko fix ' : function () { - var editorReady = false; - function onEditorReady() { var inst = editor.frame.getInstance(), container = editor.frame.get('container'), @@ -747,15 +736,10 @@ YUI.add('editor-tests', function(Y) { 'selection focusNode wrong'); Y.Assert.areEqual(text.length, sel.anchorOffset, 'selection anchorOffset wrong'); - editorReady = true; } this.makeEditorWithParaPlugin('

content

', onEditorReady); - - //If editor isn't ready within 3s, fail. - if (!editorReady) { - this.wait(3000); - } + this.wait(); }, 'test_double_plug_setup ': function() { From 3dfd41fd34d78dc3510de4cede6f683fe3b6ac65 Mon Sep 17 00:00:00 2001 From: Joe Covalesky Date: Wed, 5 Feb 2014 22:08:33 -0800 Subject: [PATCH 5/5] tweaks per https://github.com/yui/yui3/pull/1502/files#r8852815 --- src/editor/js/editor-para.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/editor/js/editor-para.js b/src/editor/js/editor-para.js index bb456318a5a..2557f1754cd 100644 --- a/src/editor/js/editor-para.js +++ b/src/editor/js/editor-para.js @@ -249,6 +249,17 @@ } if (Y.UA.gecko) { + /* + * This forced FF to redraw the content on backspace. + * On some occasions FF will leave a cursor residue after content has been deleted. + * Dropping in the empty textnode and then removing it causes FF to redraw and + * remove the "ghost cursors" + */ + // d = e.changedNode; + // t = inst.config.doc.createTextNode(' '); + // d.appendChild(t); + // d.removeChild(t); + this._fixGeckoOnBackspace(inst); } break;