wikimedia/mediawiki-extensions-VisualEditor
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/VisualEditor synced 2024-11-23 22:13:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

(bug 37980) Cutting all text and repasting it breaks the editor

Browse source 
This was caused by a bug in fixupInsertion that caused it to believe
that inserting something like "a</p><p>b</p><p>c" into the middle of an
empty paragraph was invalid.

This commit fixes the fixupInsertion bug, which fixes the
select-all-cut-paste behavior in Chrome. It's still broken in Firefox
because of selection-related issues, but I'll split that out into a
different bug report.

Change-Id: I767f5d37ec7e511778ae9ca8283ec4b26c728298
This commit is contained in:
Catrope 2012-07-05 12:55:52 -07:00 committed by Trevor Parscal
parent d9a2a9a181
commit 13ba9b7de8
2 changed files with 71 additions and 44 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

90
modules/ve/dm/ve.dm.Document.js
View file

@ -922,7 +922,7 @@ ve.dm.Document.prototype.fixupInsertion = function( data, offset ) {
// Array of element openings (object). Openings in data are pushed onto this stack
// when they are encountered and popped off when they are closed
openingStack = [],
// Array of element types (strings). Closings in data that close nodes that were
// Array of node objects. Closings in data that close nodes that were
// not opened in data (i.e. were already in the document) are pushed onto this stack
// and popped off when balanced out by an opening in data
closingStack = [],
@ -977,18 +977,23 @@ ve.dm.Document.prototype.fixupInsertion = function( data, offset ) {
*/
function writeElement( element, index ) {
var expectedType;
if ( element.type !== undefined ) {
// Content, do nothing
if ( element.type.charAt( 0 ) !== '/' ) {
// Opening
// Check if this opening balances an earlier closing of a node
// that was already in the document. This is only the case if
// openingStack is empty (otherwise we still have unclosed nodes from
// within data) and if this opening matches the top of closingStack
// Check if this opening balances an earlier closing of a node that was already in
// the document. This is only the case if openingStack is empty (otherwise we still
// have unclosed nodes from within data) and if this opening matches the top of
// closingStack
if ( openingStack.length === 0 && closingStack.length > 0 &&
closingStack[closingStack.length - 1] === element.type
closingStack[closingStack.length - 1].getType() === element.type
) {
// The top of closingStack is now balanced out, so remove it
closingStack.pop();
// Also restore parentNode from closingStack. While this is technically not
// entirely accurate (the current node is a new node that's a sibling of this
// node), it's good enough for the purposes of this algorithm
parentNode = closingStack.pop();
} else {
// This opens something new, put it on openingStack
openingStack.push( element );
@ -998,29 +1003,28 @@ ve.dm.Document.prototype.fixupInsertion = function( data, offset ) {
// Closing
// Make sure that this closing matches the currently opened node
if ( openingStack.length > 0 ) {
// The opening was on openingStack, so we're closing
// a node that was opened within data. Don't track
// that on closingStack
// The opening was on openingStack, so we're closing a node that was opened
// within data. Don't track that on closingStack
expectedType = openingStack.pop().type;
} else {
// openingStack is empty, so we're closing a node that
// was already in the document. This means we have to
// reopen it later, so track this on closingStack
// openingStack is empty, so we're closing a node that was already in the
// document. This means we have to reopen it later, so track this on
// closingStack
expectedType = parentNode.getType();
closingStack.push( expectedType );
closingStack.push( parentNode );
parentNode = parentNode.getParent();
if ( !parentNode ) {
throw 'Inserted data is trying to close the root node ' +
'(at index ' + index + ')';
}
}
parentType = expectedType;
parentType = expectedType;
// Validate
// FIXME this breaks certain input, should fix it up, not scream and die
if ( element.type !== '/' + expectedType ) {
throw 'Type mismatch, expected /' + expectedType +
' but got ' + element.type + ' (at index ' + index + ')';
// Validate
// FIXME this breaks certain input, should fix it up, not scream and die
if ( element.type !== '/' + expectedType ) {
throw 'Type mismatch, expected /' + expectedType +
' but got ' + element.type + ' (at index ' + index + ')';
}
}
}
}
@ -1056,11 +1060,11 @@ ve.dm.Document.prototype.fixupInsertion = function( data, offset ) {
closings = [];
reopenElements = [];
// Opening or content
// Make sure that opening this element here does not violate the
// parent/children/content rules. If it does, insert stuff to fix it
// Make sure that opening this element here does not violate the parent/children/content
// rules. If it does, insert stuff to fix it
// If this node is content, check that the containing node can contain
// content. If not, wrap in a paragraph
// If this node is content, check that the containing node can contain content. If not,
// wrap in a paragraph
if ( ve.dm.nodeFactory.isNodeContent( childType ) &&
!ve.dm.nodeFactory.canNodeContainContent( parentType )
) {
@ -1068,8 +1072,8 @@ ve.dm.Document.prototype.fixupInsertion = function( data, offset ) {
openings.unshift ( { 'type': 'paragraph' } );
}
// Check that this node is allowed to have the containing node as its
// parent. If not, wrap it until it's fixed
// Check that this node is allowed to have the containing node as its parent. If not,
// wrap it until it's fixed
do {
allowedParents = ve.dm.nodeFactory.getParentNodeTypes( childType );
parentsOK = allowedParents === null ||
@ -1086,14 +1090,14 @@ ve.dm.Document.prototype.fixupInsertion = function( data, offset ) {
}
} while ( !parentsOK );
// Check that the containing node can have this node as its child. If not,
// close nodes until it's fixed
// Check that the containing node can have this node as its child. If not, close nodes
// until it's fixed
do {
allowedChildren = ve.dm.nodeFactory.getChildNodeTypes( parentType );
childrenOK = allowedChildren === null ||
$.inArray( childType, allowedChildren ) !== -1;
// Also check if we're trying to insert structure into a node that
// has to contain content
// Also check if we're trying to insert structure into a node that has to contain
// content
childrenOK = childrenOK && !(
!ve.dm.nodeFactory.isNodeContent( childType ) &&
ve.dm.nodeFactory.canNodeContainContent( parentType )
@ -1106,14 +1110,13 @@ ve.dm.Document.prototype.fixupInsertion = function( data, offset ) {
popped = openingStack.pop();
parentType = popped.type;
reopenElements.push( ve.copyObject( popped ) );
// The opening was on openingStack, so we're closing
// a node that was opened within data. Don't track
// that on closingStack
// The opening was on openingStack, so we're closing a node that was opened
// within data. Don't track that on closingStack
} else {
// openingStack is empty, so we're closing a node that
// was already in the document. This means we have to
// reopen it later, so track this on closingStack
closingStack.push( parentType );
// openingStack is empty, so we're closing a node that was already in the
// document. This means we have to reopen it later, so track this on
// closingStack
closingStack.push( parentNode );
reopenElements.push( parentNode.getClonedElement() );
parentNode = parentNode.getParent();
if ( !parentNode ) {
@ -1127,9 +1130,8 @@ ve.dm.Document.prototype.fixupInsertion = function( data, offset ) {
} while( !childrenOK );
for ( j = 0; j < closings.length; j++ ) {
// writeElement() would update openingStack/closingStack, but
// we've already done that for closings
//writeElement( closings[j], i );
// writeElement() would update openingStack/closingStack, but we've already done
// that for closings
newData.push( closings[j] );
}
for ( j = 0; j < openings.length; j++ ) {
@ -1148,8 +1150,8 @@ ve.dm.Document.prototype.fixupInsertion = function( data, offset ) {
'reopenElements': reopenElements
} );
}
// If we didn't wrap the text node, then the node we're inserting
// into can have content, so we couldn't have closed anything
// If we didn't wrap the text node, then the node we're inserting into can have
// content, so we couldn't have closed anything
} else {
fixupStack.push( {
'expectedType': '/' + data[i].type,
@ -1213,7 +1215,7 @@ ve.dm.Document.prototype.fixupInsertion = function( data, offset ) {
popped = closingStack[closingStack.length - 1];
// writeElement() will perform the actual pop() that removes
// popped from closingStack
writeElement( { 'type': popped }, i );
writeElement( { 'type': popped.getType() }, i );
}
return newData;

25
tests/ve/dm/ve.dm.Transaction.test.js
View file

@ -42,6 +42,7 @@ ve.dm.Transaction.runConstructorTests = function( constructor, cases ) {
test( 'newFromInsertion', function() {
var doc = new ve.dm.Document( ve.dm.example.data ),
doc2 = new ve.dm.Document( [ { 'type': 'paragraph' }, { 'type': '/paragraph' } ] ),
cases = {
'paragraph before first element': {
'args': [doc, 0, [{ 'type': 'paragraph' }, '1', { 'type': '/paragraph' }]],
@ -173,6 +174,30 @@ test( 'newFromInsertion', function() {
},
{ 'type': 'retain', 'length': 27 }
]
},
'inserting two paragraphs into a document with just an empty paragraph': {
'args': [doc2, 1, ['F', 'O', 'O', { 'type': '/paragraph' }, { 'type': 'paragraph' }, 'B', 'A', 'R']],
'ops': [
{ 'type': 'retain', 'length': 1 },
{
'type': 'replace',
'remove': [],
'insert': ['F', 'O', 'O', { 'type': '/paragraph' }, { 'type': 'paragraph' }, 'B', 'A', 'R']
},
{ 'type': 'retain', 'length': 1 }
]
},
'inserting three paragraphs into a document with just an empty paragraph': {
'args': [doc2, 1, ['F', 'O', 'O', { 'type': '/paragraph' }, { 'type': 'paragraph' }, 'B', 'A', 'R', { 'type': '/paragraph' }, { 'type': 'paragraph' }, 'B', 'A', 'Z']],
'ops': [
{ 'type': 'retain', 'length': 1 },
{
'type': 'replace',
'remove': [],
'insert': ['F', 'O', 'O', { 'type': '/paragraph' }, { 'type': 'paragraph' }, 'B', 'A', 'R', { 'type': '/paragraph' }, { 'type': 'paragraph' }, 'B', 'A', 'Z']
},
{ 'type': 'retain', 'length': 1 }
]
}
// TODO test cases for unclosed openings
// TODO test cases for (currently failing) unopened closings use case