mirror of
https://gerrit.wikimedia.org/r/mediawiki/extensions/VisualEditor
synced 2024-09-25 11:16:51 +00:00
No longer create zero-length text nodes
We were populating empty content nodes with zero-length text nodes to make round-trip tests in the test suite work (otherwise blanking a paragraph leaves behind a zero-length text node whereas creating an empty paragraph does not), but the empty nodes are causing problems in CE apparently. * Do not create empty text nodes when constructing a node tree * Be more careful with text-only replacements: ** Don't resize a text node to zero, remove it instead ** There may not be a text node to resize at all, build it in that case ** Switch nodeRange to nodeOuterRange, this was probably broken before Tests: * Change test case for zero-length text node to assert that there is *no* zero-length text node :) * Remove a test case concerning an empty text node from the ve.ce.TextNode suite Change-Id: Ie677457f2f0a7823a517ba3077b844ef52a20fcc
This commit is contained in:
parent
580ed725a7
commit
7702ec10dc
|
@ -106,14 +106,6 @@ ve.dm.Document = function VeDmDocument( data, parentDocument ) {
|
|||
throw new Error( 'Unbalanced input passed to document' );
|
||||
}
|
||||
|
||||
if ( children.length === 0 &&
|
||||
ve.dm.nodeFactory.canNodeContainContent(
|
||||
currentNode.getType()
|
||||
)
|
||||
) {
|
||||
// Content nodes cannot be childless, add a zero-length text node
|
||||
children.push( new ve.dm.TextNode( 0 ) );
|
||||
}
|
||||
// Attach the children to the node
|
||||
ve.batchSplice( currentNode, 0, 0, children );
|
||||
}
|
||||
|
|
|
@ -224,21 +224,26 @@ ve.dm.TransactionProcessor.processors.replace = function ( op ) {
|
|||
),
|
||||
'leaves'
|
||||
);
|
||||
if ( removeHasStructure || insertHasStructure ) {
|
||||
// Replacement is not exclusively text
|
||||
firstNode = selection[0].node;
|
||||
if (
|
||||
// Replacement is text only
|
||||
!removeHasStructure && !insertHasStructure &&
|
||||
// firstNode is a text node
|
||||
firstNode.getType() === 'text' &&
|
||||
// We're not blanking the text node
|
||||
firstNode.getLength() + insert.length - remove.length > 0
|
||||
) {
|
||||
// Resize firstNode
|
||||
this.synchronizer.pushResize( firstNode, insert.length - remove.length );
|
||||
} else {
|
||||
// Rebuild all covered nodes
|
||||
range = new ve.Range(
|
||||
selection[0].nodeRange.start, selection[selection.length - 1].nodeRange.end
|
||||
selection[0].nodeOuterRange.start, selection[selection.length - 1].nodeOuterRange.end
|
||||
);
|
||||
this.synchronizer.pushRebuild( range,
|
||||
new ve.Range( range.start + this.adjustment,
|
||||
range.end + this.adjustment + insert.length - remove.length )
|
||||
);
|
||||
} else {
|
||||
// Text-only replacement
|
||||
// Queue a resize for this node
|
||||
node = selection[0].node;
|
||||
this.synchronizer.pushResize( node, insert.length - remove.length );
|
||||
}
|
||||
// Advance the cursor
|
||||
this.cursor += insert.length;
|
||||
|
@ -270,16 +275,7 @@ ve.dm.TransactionProcessor.processors.replace = function ( op ) {
|
|||
prevCursor + opRemove.length - this.adjustment
|
||||
), 'siblings' );
|
||||
for ( i = 0; i < selection.length; i++ ) {
|
||||
// .nodeRange is the inner range, we need the
|
||||
// outer range (including opening and closing)
|
||||
if ( selection[i].node.isWrapped() ) {
|
||||
affectedRanges.push( new ve.Range(
|
||||
selection[i].nodeRange.start - 1,
|
||||
selection[i].nodeRange.end + 1
|
||||
) );
|
||||
} else {
|
||||
affectedRanges.push( selection[i].nodeRange );
|
||||
}
|
||||
affectedRanges.push( selection[i].nodeOuterRange );
|
||||
}
|
||||
}
|
||||
// Walk through the remove and insert data
|
||||
|
|
|
@ -9,7 +9,7 @@ QUnit.module( 've.ce.TextNode' );
|
|||
|
||||
/* Tests */
|
||||
|
||||
QUnit.test( 'getHtml', 22, function ( assert ) {
|
||||
QUnit.test( 'getHtml', function ( assert ) {
|
||||
var i, len, cases;
|
||||
|
||||
cases = [
|
||||
|
@ -213,11 +213,6 @@ QUnit.test( 'getHtml', 22, function ( assert ) {
|
|||
],
|
||||
'html': 'abc<i><u><b>d</b></u></i><u><b>ef</b></u>ghi'
|
||||
},
|
||||
{
|
||||
// [ ]
|
||||
'data': [{ 'type': 'paragraph' },{ 'type': '/paragraph' }],
|
||||
'html': ''
|
||||
},
|
||||
{
|
||||
// [ ]
|
||||
'data': [{ 'type': 'paragraph' },' ',{ 'type': '/paragraph' }],
|
||||
|
@ -274,6 +269,7 @@ QUnit.test( 'getHtml', 22, function ( assert ) {
|
|||
'html': ' A'
|
||||
}
|
||||
];
|
||||
expect( cases.length );
|
||||
for ( i = 0, len = cases.length; i < len; i++ ) {
|
||||
ve.dm.example.preprocessAnnotations( cases[i].data );
|
||||
assert.equal(
|
||||
|
|
|
@ -34,8 +34,8 @@ QUnit.test( 'constructor', 4, function ( assert ) {
|
|||
doc = new ve.dm.Document( [ { 'type': 'paragraph' }, { 'type': '/paragraph' } ] );
|
||||
assert.equalNodeTree(
|
||||
doc.getDocumentNode(),
|
||||
new ve.dm.DocumentNode( [ new ve.dm.ParagraphNode( [ new ve.dm.TextNode( 0 ) ] ) ] ),
|
||||
'empty paragraph gets a zero-length text node'
|
||||
new ve.dm.DocumentNode( [ new ve.dm.ParagraphNode( [ ] ) ] ),
|
||||
'empty paragraph does not get a zero-length text node'
|
||||
);
|
||||
} );
|
||||
|
||||
|
|
Loading…
Reference in a new issue