From e89e9910377218c8593fe7a55ca7e9123dcd4b6a Mon Sep 17 00:00:00 2001 From: Roan Kattouw Date: Fri, 26 Apr 2013 16:49:43 -0700 Subject: [PATCH] Paragraph->heading conversion was broken when cursor next to an inline node In this case, selectNodes() returns the paragraph, which is not a content node, and so newFromContentBranchConversion() decides to do nothing at all. This change makes newFromContentBranchConversion() more intelligent about finding the content branch to operate on, fixing cases such as these where selectNodes() returns the content branch itself rather than one of its children. Bug: 41203 Change-Id: I710fbf184ef5ef84d9c2f5bca2b115e0660f5b8f --- modules/ve/dm/ve.dm.Transaction.js | 4 +- modules/ve/test/dm/ve.dm.Transaction.test.js | 97 ++++++++++++++++++++ modules/ve/test/dm/ve.dm.example.js | 17 ++++ 3 files changed, 116 insertions(+), 2 deletions(-) diff --git a/modules/ve/dm/ve.dm.Transaction.js b/modules/ve/dm/ve.dm.Transaction.js index d8ce506c0e..6aa3319106 100644 --- a/modules/ve/dm/ve.dm.Transaction.js +++ b/modules/ve/dm/ve.dm.Transaction.js @@ -396,8 +396,8 @@ ve.dm.Transaction.newFromContentBranchConversion = function ( doc, range, type, // Replace the wrappings of each content branch in the range for ( i = 0; i < selection.length; i++ ) { selected = selection[i]; - if ( selected.node.isContent() ) { - branch = selected.node.getParent(); + branch = selected.node.isContent() ? selected.node.getParent() : selected.node; + if ( branch.canContainContent() ) { // Skip branches that are already of the target type and have identical attributes if ( branch.getType() === type && ve.compareObjects( branch.getAttributes(), attr ) ) { continue; diff --git a/modules/ve/test/dm/ve.dm.Transaction.test.js b/modules/ve/test/dm/ve.dm.Transaction.test.js index 9189f47c7c..6550974b44 100644 --- a/modules/ve/test/dm/ve.dm.Transaction.test.js +++ b/modules/ve/test/dm/ve.dm.Transaction.test.js @@ -800,6 +800,7 @@ QUnit.test( 'newFromAnnotation', function ( assert ) { QUnit.test( 'newFromContentBranchConversion', function ( assert ) { var i, key, store, doc = ve.dm.example.createExampleDocument(), + doc2 = ve.dm.example.createExampleDocument( 'inlineAtEdges' ), cases = { 'range inside a heading, convert to paragraph': { 'args': [doc, new ve.Range( 1, 2 ), 'paragraph'], @@ -847,6 +848,102 @@ QUnit.test( 'newFromContentBranchConversion', function ( assert ) { }, { 'type': 'retain', 'length': 3 } ] + }, + 'zero-length range before inline node at the start': { + 'args': [doc2, new ve.Range( 1, 1 ), 'heading', { 'level': 2 }], + 'ops': [ + { + 'type': 'replace', + 'remove': [{ 'type': 'paragraph' }], + 'insert': [{ 'type': 'heading', 'attributes': { 'level': 2 } }] + }, + { 'type': 'retain', 'length': 7 }, + { + 'type': 'replace', + 'remove': [{ 'type': '/paragraph' }], + 'insert': [{ 'type': '/heading' }] + } + ] + }, + 'zero-length range inside inline node at the start': { + 'args': [doc2, new ve.Range( 2, 2 ), 'heading', { 'level': 2 }], + 'ops': [ + { + 'type': 'replace', + 'remove': [{ 'type': 'paragraph' }], + 'insert': [{ 'type': 'heading', 'attributes': { 'level': 2 } }] + }, + { 'type': 'retain', 'length': 7 }, + { + 'type': 'replace', + 'remove': [{ 'type': '/paragraph' }], + 'insert': [{ 'type': '/heading' }] + } + ] + }, + 'zero-length range after inline node at the start': { + 'args': [doc2, new ve.Range( 3, 3 ), 'heading', { 'level': 2 }], + 'ops': [ + { + 'type': 'replace', + 'remove': [{ 'type': 'paragraph' }], + 'insert': [{ 'type': 'heading', 'attributes': { 'level': 2 } }] + }, + { 'type': 'retain', 'length': 7 }, + { + 'type': 'replace', + 'remove': [{ 'type': '/paragraph' }], + 'insert': [{ 'type': '/heading' }] + } + ] + }, + 'zero-length range before inline node at the end': { + 'args': [doc2, new ve.Range( 6, 6 ), 'heading', { 'level': 2 }], + 'ops': [ + { + 'type': 'replace', + 'remove': [{ 'type': 'paragraph' }], + 'insert': [{ 'type': 'heading', 'attributes': { 'level': 2 } }] + }, + { 'type': 'retain', 'length': 7 }, + { + 'type': 'replace', + 'remove': [{ 'type': '/paragraph' }], + 'insert': [{ 'type': '/heading' }] + } + ] + }, + 'zero-length range inside inline node at the end': { + 'args': [doc2, new ve.Range( 7, 7 ), 'heading', { 'level': 2 }], + 'ops': [ + { + 'type': 'replace', + 'remove': [{ 'type': 'paragraph' }], + 'insert': [{ 'type': 'heading', 'attributes': { 'level': 2 } }] + }, + { 'type': 'retain', 'length': 7 }, + { + 'type': 'replace', + 'remove': [{ 'type': '/paragraph' }], + 'insert': [{ 'type': '/heading' }] + } + ] + }, + 'zero-length range after inline node at the end': { + 'args': [doc2, new ve.Range( 8, 8 ), 'heading', { 'level': 2 }], + 'ops': [ + { + 'type': 'replace', + 'remove': [{ 'type': 'paragraph' }], + 'insert': [{ 'type': 'heading', 'attributes': { 'level': 2 } }] + }, + { 'type': 'retain', 'length': 7 }, + { + 'type': 'replace', + 'remove': [{ 'type': '/paragraph' }], + 'insert': [{ 'type': '/heading' }] + }, + ] } }; QUnit.expect( ve.getObjectKeys( cases ).length ); diff --git a/modules/ve/test/dm/ve.dm.example.js b/modules/ve/test/dm/ve.dm.example.js index 01086f50aa..6c8de4ac3f 100644 --- a/modules/ve/test/dm/ve.dm.example.js +++ b/modules/ve/test/dm/ve.dm.example.js @@ -627,6 +627,23 @@ ve.dm.example.complexTable = [ { 'type': '/table' } ]; +ve.dm.example.inlineAtEdges = [ + { 'type': 'paragraph' }, + { 'type': 'image', 'attributes': { + 'html/0/src': ve.dm.example.imgSrc, + 'src': ve.dm.example.imgSrc, + 'width': null, + 'height': null + } }, + { 'type': '/image' }, + 'F', + 'o', + 'o', + { 'type': 'alienInline', 'attributes': { 'html': '' } }, + { 'type': '/alienInline' }, + { 'type': '/paragraph' } +]; + /** * Sample content data index. *