Merge "Prevent deletion of FocusableNodes from a collapsed selection"

This commit is contained in:
jenkins-bot 2013-10-09 17:22:45 +00:00 committed by Gerrit Code Review
commit 39d5a1a9c4
5 changed files with 167 additions and 50 deletions

View file

@ -356,6 +356,7 @@ class VisualEditorHooks {
've/test/ce/ve.ce.Document.test.js',
've/test/ce/ve.ce.Surface.test.js',
've-mw/test/ce/ve.ce.Document.test.js',
've-mw/test/ce/ve.ce.Surface.test.js',
've/test/ce/ve.ce.NodeFactory.test.js',
've/test/ce/ve.ce.Node.test.js',
've/test/ce/ve.ce.BranchNode.test.js',

View file

@ -0,0 +1,60 @@
/*!
* VisualEditor ContentEditable Surface tests.
*
* @copyright 2011-2013 VisualEditor Team and others; see AUTHORS.txt
* @license The MIT License (MIT); see LICENSE.txt
*/
QUnit.module( 've.ce.Surface' );
/* Tests */
QUnit.test( 'handleDelete', function ( assert ) {
var i,
cases = [
{
'html':
'<p>Foo</p>' +
ve.dm.mwExample.MWTransclusion.blockOpen + ve.dm.mwExample.MWTransclusion.blockContent +
'<p>Bar</p>',
'range': new ve.Range( 4 ),
'operations': ['delete'],
'expectedData': function () {},
'expectedRange': new ve.Range( 5, 7 ),
'msg': 'Block transclusion is focused not deleted'
},
{
'html':
'<p>Foo</p>' +
ve.dm.mwExample.MWTransclusion.blockOpen + ve.dm.mwExample.MWTransclusion.blockContent +
'<p>Bar</p>',
'range': new ve.Range( 4 ),
'operations': ['delete', 'delete'],
'expectedData': function ( data ) {
data.splice( 5, 2 );
},
'expectedRange': new ve.Range( 5, 5 ),
'msg': 'Block transclusion is deleted with two keypresses'
},
{
'html':
'<p>Foo</p>' +
ve.dm.mwExample.MWBlockImage.html +
'<p>Bar</p>',
'range': new ve.Range( 4 ),
'operations': ['delete'],
'expectedData': function () { },
'expectedRange': new ve.Range( 5, 14 ),
'msg': 'Block image is focused not deleted'
}
];
QUnit.expect( cases.length * 2 );
for ( i = 0; i < cases.length; i++ ) {
ve.test.utils.runSurfaceHandleDeleteTest(
assert, cases[i].html, cases[i].range, cases[i].operations,
cases[i].expectedData, cases[i].expectedRange, cases[i].msg
);
}
} );

View file

@ -126,6 +126,37 @@ ve.dm.mwExample.MWTransclusion.mixedStoreItems = {
'value': $( ve.dm.mwExample.MWTransclusion.mixed ).toArray()
};
ve.dm.mwExample.MWBlockImage = {
'html':
'<figure typeof="mw:Image/Thumb" class="mw-halign-right foobar">' +
'<a href="Foo"><img src="Bar" width="1" height="2" resource="FooBar"></a>' +
'<figcaption>abc</figcaption>' +
'</figure>',
'data': [
{
'type': 'mwBlockImage',
'attributes': {
'type': 'thumb',
'align': 'right',
'href': 'Foo',
'src': 'Bar',
'width': '1',
'height': '2',
'resource': 'FooBar',
'originalClasses': 'mw-halign-right foobar',
'unrecognizedClasses': ['foobar']
}
},
{ 'type': 'mwImageCaption' },
{ 'type': 'paragraph', 'internal': { 'generated': 'wrapper' } },
'a', 'b', 'c',
{ 'type': '/paragraph' },
{ 'type': '/mwImageCaption' },
{ 'type': '/mwBlockImage' }
]
};
ve.dm.mwExample.MWReference = {
'referenceList':
'<ol class="references" typeof="mw:Extension/references" about="#mwt7" data-parsoid="{}"' +
@ -1613,31 +1644,11 @@ ve.dm.mwExample.domToDataCases = {
]
},
'thumb image': {
'html': '<body><figure typeof="mw:Image/Thumb" class="mw-halign-right foobar"><a href="Foo"><img src="Bar" width="1" height="2" resource="FooBar"></a><figcaption>abc</figcaption></figure></body>',
'data': [
{
'type': 'mwBlockImage',
'attributes': {
'type': 'thumb',
'align': 'right',
'href': 'Foo',
'src': 'Bar',
'width': '1',
'height': '2',
'resource': 'FooBar',
'originalClasses': 'mw-halign-right foobar',
'unrecognizedClasses': ['foobar']
}
},
{ 'type': 'mwImageCaption' },
{ 'type': 'paragraph', 'internal': { 'generated': 'wrapper' } },
'a', 'b', 'c',
{ 'type': '/paragraph' },
{ 'type': '/mwImageCaption' },
{ 'type': '/mwBlockImage' },
'html': '<body>' + ve.dm.mwExample.MWBlockImage.html + '</body>',
'data': ve.dm.mwExample.MWBlockImage.data.concat( [
{ 'type': 'internalList' },
{ 'type': '/internalList' }
]
] )
},
'attribute preservation does not crash due to text node split': {
'html':

View file

@ -1512,7 +1512,8 @@ ve.ce.Surface.prototype.handleEnter = function ( e ) {
*/
ve.ce.Surface.prototype.handleDelete = function ( e, backspace ) {
var rangeToRemove = this.model.getSelection(),
tx, startNode, endNode, endNodeData, nodeToDelete;
offset = 0,
docLength, tx, startNode, endNode, endNodeData, nodeToDelete;
if ( rangeToRemove.isCollapsed() ) {
// In case when the range is collapsed use the same logic that is used for cursor left and
@ -1523,6 +1524,20 @@ ve.ce.Surface.prototype.handleDelete = function ( e, backspace ) {
( e.altKey === true || e.ctrlKey === true ) ? 'word' : 'character',
true
);
offset = rangeToRemove.start;
docLength = this.model.getDocument().data.getLength();
if ( offset < docLength ) {
while ( offset < docLength && this.model.getDocument().data.isCloseElementData( offset ) ) {
offset++;
}
// If the user tries to delete a focusable node from a collapsed selection,
// just select the node and cancel the deletion.
startNode = this.documentView.getDocumentNode().getNodeFromOffset( offset + 1 );
if ( ve.isMixedIn( startNode, ve.ce.FocusableNode ) ) {
this.model.change( null, startNode.getModel().getOuterRange() );
return;
}
}
if ( rangeToRemove.isCollapsed() ) {
// For instance beginning or end of the document.
return;

View file

@ -9,19 +9,41 @@ QUnit.module( 've.ce.Surface' );
/* Tests */
QUnit.test( 'handleDelete', function ( assert ) {
var i,
surface = ve.test.utils.createSurfaceFromHtml( ve.dm.example.html ),
view = surface.getView(),
model = surface.getModel(),
data = ve.copy( model.getDocument().getFullData() ),
originalData = ve.copy( data ),
ve.test.utils.runSurfaceHandleDeleteTest = function( assert, html, range, operations, expectedData, expectedRange, msg ) {
var i, args,
selection,
deleteArgs = {
'backspace': [ {}, true ],
'delete': [ {}, false ],
'modifiedBackspace': [ { 'ctrlKey': true }, true ],
'modifiedDelete': [ { 'ctrlKey': true }, false ]
},
surface = ve.test.utils.createSurfaceFromHtml( html || ve.dm.example.html ),
view = surface.getView(),
model = surface.getModel(),
data = ve.copy( model.getDocument().getFullData() );
// TODO: model.getSelection() should be consistent after it has been
// changed but appears to behave differently depending on the browser.
// The selection from the change event is still consistent.
model.on( 'change', function( tx, s ) {
selection = s;
} );
model.change( null, range );
for ( i = 0; i < operations.length; i++ ) {
args = deleteArgs[operations[i]];
view.handleDelete( args[0], args[1] );
}
expectedData( data );
assert.deepEqualWithDomElements( model.getDocument().getFullData(), data, msg + ': data' );
assert.deepEqual( selection, expectedRange, msg + ': range' );
surface.destroy();
};
QUnit.test( 'handleDelete', function ( assert ) {
var i,
cases = [
{
'range': new ve.Range( 2 ),
@ -85,31 +107,39 @@ QUnit.test( 'handleDelete', function ( assert ) {
},
'expectedRange': new ve.Range( 1 ),
'msg': 'Empty node deleted by delete'
},
{
'range': new ve.Range( 41 ),
'operations': ['backspace'],
'expectedData': function () {},
'expectedRange': new ve.Range( 39, 41 ),
'msg': 'Focusable node selected but not deleted by backspace'
},
{
'range': new ve.Range( 39 ),
'operations': ['delete'],
'expectedData': function () {},
'expectedRange': new ve.Range( 39, 41 ),
'msg': 'Focusable node selected but not deleted by delete'
},
{
'range': new ve.Range( 39, 41 ),
'operations': ['delete'],
'expectedData': function ( data ) {
data.splice( 39, 2 );
},
'expectedRange': new ve.Range( 39 ),
'msg': 'Focusable node deleted if selected first'
}
];
function testRunner( range, operations, expectedData, expectedRange, msg ) {
var i, args, data = ve.copy( originalData );
model.change( null, range );
for ( i = 0; i < operations.length; i++ ) {
args = deleteArgs[operations[i]];
view.handleDelete( args[0], args[1] );
}
expectedData( data );
assert.deepEqual( model.getDocument().getFullData(), data, msg + ': data' );
assert.deepEqual( model.getSelection(), expectedRange, msg + ': range' );
// Roll back the test Surface
while ( model.undo() ) {
/*jshint noempty:false */
}
}
QUnit.expect( cases.length * 2 );
for ( i = 0; i < cases.length; i++ ) {
testRunner( cases[i].range, cases[i].operations, cases[i].expectedData, cases[i].expectedRange, cases[i].msg );
ve.test.utils.runSurfaceHandleDeleteTest(
assert, cases[i].html, cases[i].range, cases[i].operations,
cases[i].expectedData, cases[i].expectedRange, cases[i].msg
);
}
} );