Esanders has uploaded a new change for review.
https://gerrit.wikimedia.org/r/98515
Change subject: Fix deletion inside a structural node and start/end of document
......................................................................
Fix deletion inside a structural node and start/end of document
Logic assumed that if you had a collapsed selection near the start
or end of the document that you didn't want to delete anything,
however this is only true for content nodes. Structural nodes
should be unwrapped when delete is pressed.
Bug: 50250
Change-Id: I643449c1fa08ea12c8c3aa13f4a4b97d8876990d
---
M modules/ve/ce/ve.ce.Surface.js
M modules/ve/test/ce/ve.ce.Surface.test.js
M modules/ve/ve.BranchNode.js
3 files changed, 43 insertions(+), 8 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/15/98515/1
diff --git a/modules/ve/ce/ve.ce.Surface.js b/modules/ve/ce/ve.ce.Surface.js
index 7f7a57c..330bc4b 100644
--- a/modules/ve/ce/ve.ce.Surface.js
+++ b/modules/ve/ce/ve.ce.Surface.js
@@ -1678,9 +1678,9 @@
* @param {boolean} backspace Key was a backspace
*/
ve.ce.Surface.prototype.handleDelete = function ( e, backspace ) {
- var rangeToRemove = this.model.getSelection(),
- offset = 0,
- docLength, tx, startNode, endNode, endNodeData, nodeToDelete;
+ var docLength, tx, startNode, endNode, endNodeData, nodeToDelete,
+ nodeRange, offset,
+ rangeToRemove = this.model.getSelection();
if ( rangeToRemove.isCollapsed() ) {
// In case when the range is collapsed use the same logic that
is used for cursor left and
@@ -1692,9 +1692,9 @@
true
);
offset = rangeToRemove.start;
- docLength = this.model.getDocument().data.getLength();
- if ( offset < docLength ) {
- while ( offset < docLength &&
this.model.getDocument().data.isCloseElementData( offset ) ) {
+ docLength =
this.model.getDocument().getInternalList().getListNode().getOuterRange().start;
+ if ( offset < docLength - 1 ) {
+ while ( offset < docLength - 1 &&
this.model.getDocument().data.isCloseElementData( offset ) ) {
offset++;
}
// If the user tries to delete a focusable node from a
collapsed selection,
@@ -1706,8 +1706,17 @@
}
}
if ( rangeToRemove.isCollapsed() ) {
- // For instance beginning or end of the document.
- return;
+ startNode =
this.documentView.getDocumentNode().getNodeFromOffset( offset );
+ nodeRange = startNode.getModel().getOuterRange();
+ if (
+ startNode.canContainContent() &&
+ ( nodeRange.start === 0 || nodeRange.end ===
docLength )
+ ) {
+ // Content node at start or end of document, do
nothing.
+ return;
+ } else {
+ rangeToRemove = new ve.Range( nodeRange.start,
rangeToRemove.start - 1);
+ }
}
}
tx = ve.dm.Transaction.newFromRemoval( this.documentView.model,
rangeToRemove );
diff --git a/modules/ve/test/ce/ve.ce.Surface.test.js
b/modules/ve/test/ce/ve.ce.Surface.test.js
index 39259da..e4e5b88 100644
--- a/modules/ve/test/ce/ve.ce.Surface.test.js
+++ b/modules/ve/test/ce/ve.ce.Surface.test.js
@@ -45,6 +45,7 @@
QUnit.test( 'handleDelete', function ( assert ) {
var i,
+ emptyList = '<ul><li><p></p></li></ul>',
cases = [
{
'range': new ve.Range( 2 ),
@@ -131,6 +132,28 @@
},
'expectedRange': new ve.Range( 39 ),
'msg': 'Focusable node deleted if selected
first'
+ },
+ {
+ 'html': emptyList,
+ 'range': new ve.Range( 3 ),
+ 'operations': ['backspace'],
+ 'expectedData': function ( data ) {
+ data.splice( 0, 2 );
+ data.splice( 2, 2 );
+ },
+ 'expectedRange': new ve.Range( 0 ),
+ 'msg': 'List and start of document unwrapped by
backspace'
+ },
+ {
+ 'html': emptyList,
+ 'range': new ve.Range( 3 ),
+ 'operations': ['delete'],
+ 'expectedData': function ( data ) {
+ data.splice( 0, 2 );
+ data.splice( 2, 2 );
+ },
+ 'expectedRange': new ve.Range( 0 ),
+ 'msg': 'List and end of document unwrapped by
delete'
}
];
diff --git a/modules/ve/ve.BranchNode.js b/modules/ve/ve.BranchNode.js
index dfa14ed..0894862 100644
--- a/modules/ve/ve.BranchNode.js
+++ b/modules/ve/ve.BranchNode.js
@@ -115,6 +115,9 @@
nodeOffset = 0;
for ( i = 0, length = this.children.length; i < length; i++ ) {
childNode = this.children[i];
+ if ( childNode instanceof ve.ce.InternalListNode ) {
+ break;
+ }
if ( offset === nodeOffset ) {
// The requested offset is right before
childNode,
// so it's not inside any of this's children,
but inside this
--
To view, visit https://gerrit.wikimedia.org/r/98515
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I643449c1fa08ea12c8c3aa13f4a4b97d8876990d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Esanders <esand...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
