Divec has uploaded a new change for review.
https://gerrit.wikimedia.org/r/89369
Change subject: WIP:Use native left/right/backspace/delete within text
......................................................................
WIP:Use native left/right/backspace/delete within text
TODO: Need to revisit the rerendering requirements on backspace/delete
within text
modules/ve/ce/ve.ce.Surface.js
* Native left/right cursoring inside a text node (or consecutive text nodes)
* Native backspace/delete
* Document the purpose of handleLeftOrRightArrowKey
Change-Id: I7cc1c3d91610f4dea1fd7e5b1b8b4bde334aad1f
---
M modules/ve/ce/ve.ce.Surface.js
1 file changed, 84 insertions(+), 9 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/69/89369/1
diff --git a/modules/ve/ce/ve.ce.Surface.js b/modules/ve/ce/ve.ce.Surface.js
index bf24cd9..7b73a6e 100644
--- a/modules/ve/ce/ve.ce.Surface.js
+++ b/modules/ve/ce/ve.ce.Surface.js
@@ -544,11 +544,9 @@
this.handleEnter( e );
break;
case ve.Keys.BACKSPACE:
- e.preventDefault();
this.handleDelete( e, true );
break;
case ve.Keys.DELETE:
- e.preventDefault();
this.handleDelete( e, false );
break;
default:
@@ -1081,6 +1079,7 @@
}
this.incRenderLock();
try {
+ // TODO: revisit rerendering requirements
this.model.change(
ve.dm.Transaction.newFromRemoval(
this.documentView.model,
range ),
@@ -1145,15 +1144,17 @@
}
if ( data.length > 0 ) {
- this.model.change(
- ve.dm.Transaction.newFromInsertion(
- this.documentView.model, nodeOffset + 1
+ fromLeft,
- data
- ),
- newRange
- );
+ // TODO: revisit rerendering requirements
+ this.model.change(
+ ve.dm.Transaction.newFromInsertion(
+ this.documentView.model, nodeOffset + 1 +
fromLeft,
+ data
+ ),
+ newRange
+ );
}
if ( fromLeft + fromRight < previousData.length ) {
+ // TODO: revisit rerendering requirements
this.model.change(
ve.dm.Transaction.newFromRemoval(
this.documentView.model,
@@ -1218,7 +1219,18 @@
/*! Utilities */
/**
+ * At times we can't rely on the browser's native Left/Right handling. For
instance:
+ * * To prevent cursoring into an image caption
+ * * Or into a template in IE (which does not respect contenteditable=false
inside contenteditable=true)
+ * * To allow selection of elements (e.g. placing the cursor "on image")
+ * * To prevent the cursor disappearing on some browsers e.g. in a table
before the first tr
+ * * To handle backwards selection in IE (which cannot be set
programmatically, so needs faking)
+ * * To give more consistency between browsers
+ *
+ * On the other hand, the browser's native handling is positively required to
handle cursoring
+ * over multi-codepoint Indic grapheme clusters, combining accents etc.
* @method
+ * @param {jQuery.Event} e Key down event
*/
ve.ce.Surface.prototype.handleLeftOrRightArrowKey = function ( e ) {
var selection, range, direction;
@@ -1226,6 +1238,13 @@
// As we are not able to handle it programmatically (because we don't
know at which offsets
// lines starts and ends) let it happen natively.
if ( e.metaKey ) {
+ return;
+ }
+
+ // If we're cursoring by character, within a text node, then use native
browser handling
+ // in order to handle grapheme clusters correctly.
+ if ( ! e.altKey && ! e.ctrlKey && this.willMoveInsideTextNode(
+ e.keyCode === ve.Keys.LEFT ? 'backward' : 'forward' ) ) {
return;
}
@@ -1258,6 +1277,55 @@
// TODO: onDocumentKeyDown does this anyway
this.surfaceObserver.startTimerLoop();
this.surfaceObserver.pollOnce();
+};
+
+/**
+ * Tests whether the moving cursor will stay inside the text node.
+ *
+ * Assumes the cursor will move by one character (the exact meaning of which
is potentially
+ * browser- and font-specific). Will also return true if the cursor will move
between adjacent
+ * text nodes, as some browsers will create these.
+ *
+ * @method
+ * @param {string} direction Direction of motion; 'forward' or 'backward'
+ * @returns {boolean} Whether the cursor won't enter or cross a non-text node
+ */
+ve.ce.Surface.prototype.willMoveInsideTextNode = function ( direction ) {
+ var range, node, backward = direction === 'backward',
+ sel = rangy.getSelection( this.getElementDocument() );
+ if ( sel.rangeCount === 0 ) {
+ return false;
+ }
+
+ // If moving backwards, get first range; if moving forwards, get last
range
+ if ( backward ) {
+ range = sel.getRangeAt( 0 );
+ node = range.startContainer;
+ } else {
+ range = sel.getRangeAt( sel.rangeCount - 1 );
+ node = range.endContainer;
+ }
+
+ if ( node.nodeType !== node.TEXT_NODE ) {
+ return false;
+ }
+
+ // Kludge: in Chrome, cursoring can split text nodes in two, so check
carefully. Note
+ // we can't just normalize() the parent node, because that changes the
selection state.
+ if ( range.startOffset === 0 && node.previousSibling !== null &&
+ node.previousSibling.nodeType === node.TEXT_NODE ) {
+ return true;
+ }
+ if ( range.endOffset === node.nodeValue.length && node.nextSibling !==
null &&
+ node.nextSibling.nodeType === node.TEXT_NODE ) {
+ return true;
+ }
+
+ if ( backward ) {
+ return range.startOffset > 0;
+ } else {
+ return range.endOffset < node.nodeValue.length;
+ }
};
/**
@@ -1517,6 +1585,13 @@
docLength, tx, startNode, endNode, endNodeData, nodeToDelete;
if ( rangeToRemove.isCollapsed() ) {
+ if ( ! e.altKey && ! e.ctrlKey && this.willMoveInsideTextNode(
+ backspace ? 'backward' : 'forward' ) ) {
+ // handle natively
+ return;
+ }
+ e.preventDefault();
+
// In case when the range is collapsed use the same logic that
is used for cursor left and
// right movement in order to figure out range to remove.
rangeToRemove = this.getDocument().getRelativeRange(
--
To view, visit https://gerrit.wikimedia.org/r/89369
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7cc1c3d91610f4dea1fd7e5b1b8b4bde334aad1f
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Divec <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
