jenkins-bot has submitted this change and it was merged.
Change subject: Small cleanup in CE
......................................................................
Small cleanup in CE
Better comments for:
* ve.ce.Document.getRelativeOffset,
* ve.ce.Document.getSiblingWordBoundary.
Convert ve.ce.Surface.getSelectionRect to a static method.
Moved getNodeAndOffset from ve.ce.Surface to ve.ce.Document.
Change-Id: Ic00221fa463205d04c9b52150c0dd15904493b1e
---
M modules/ve/ce/ve.ce.Document.js
M modules/ve/ce/ve.ce.Surface.js
M modules/ve/ce/ve.ce.SurfaceObserver.js
M modules/ve/ce/ve.ce.js
M modules/ve/ui/ve.ui.Context.js
M modules/ve/ve.js
6 files changed, 127 insertions(+), 133 deletions(-)
Approvals:
Catrope: Looks good to me, approved
jenkins-bot: Verified
diff --git a/modules/ve/ce/ve.ce.Document.js b/modules/ve/ce/ve.ce.Document.js
index 27e2767..67de5af 100644
--- a/modules/ve/ce/ve.ce.Document.js
+++ b/modules/ve/ce/ve.ce.Document.js
@@ -81,6 +81,8 @@
/**
* Get the nearest word boundary.
+ * This method is in CE instead of DM because its behaviour depends on the
browser (IE/non-IE) and
+ * that information is closer to view layer. (CE)
*
* @method
* @param {number} offset Offset to start from
@@ -128,6 +130,11 @@
/**
* Get the relative word or character boundary.
+ * This method is in CE instead of DM because it uses information about slugs
about which model
+ * does not know at all.
+ *
+ * FIXME: In certain cases returned offset is the same as passed offset which
prevents cursor from
+ * moving.
*
* @method
* @param {number} offset Offset to start from
@@ -158,4 +165,73 @@
return relativeContentOffset;
}
}
+};
+
+/**
+ * Get a DOM node and DOM element offset for a document offset.
+ *
+ * The results of this function are meant to be used with rangy.
+ *
+ * @method
+ * @param {number} offset Linear model offset
+ * @returns {Object} Object containing a node and offset property where node
is an HTML element and
+ * offset is the position within the element
+ * @throws {Error} Offset could not be translated to a DOM element and offset
+ */
+ve.ce.Document.prototype.getNodeAndOffset = function ( offset ) {
+ var node, startOffset, current, stack, item, $item, length,
+ $slug = this.getSlugAtOffset( offset );
+ if ( $slug ) {
+ return { node: $slug[0].childNodes[0], offset: 0 };
+ }
+ node = this.getNodeFromOffset( offset );
+ startOffset = this.getDocumentNode().getOffsetFromNode( node ) + ( (
node.isWrapped() ) ? 1 : 0 );
+ current = [node.$.contents(), 0];
+ stack = [current];
+ while ( stack.length > 0 ) {
+ if ( current[1] >= current[0].length ) {
+ stack.pop();
+ current = stack[ stack.length - 1 ];
+ continue;
+ }
+ item = current[0][current[1]];
+ if ( item.nodeType === Node.TEXT_NODE ) {
+ length = item.textContent.length;
+ if ( offset >= startOffset && offset <= startOffset +
length ) {
+ return {
+ node: item,
+ offset: offset - startOffset
+ };
+ } else {
+ startOffset += length;
+ }
+ } else if ( item.nodeType === Node.ELEMENT_NODE ) {
+ $item = current[0].eq( current[1] );
+ if ( $item.hasClass('ve-ce-slug') ) {
+ if ( offset === startOffset ) {
+ return {
+ node: $item[0],
+ offset: 1
+ };
+ }
+ } else if ( $item.is( '.ve-ce-branchNode,
.ve-ce-leafNode' ) ) {
+ length = $item.data( 'node'
).model.getOuterLength();
+ if ( offset >= startOffset && offset <
startOffset + length ) {
+ stack.push( [$item.contents(), 0] );
+ current[1]++;
+ current = stack[stack.length-1];
+ continue;
+ } else {
+ startOffset += length;
+ }
+ } else {
+ stack.push( [$item.contents(), 0] );
+ current[1]++;
+ current = stack[stack.length-1];
+ continue;
+ }
+ }
+ current[1]++;
+ }
+ throw new Error( 'Offset could not be translated to a DOM element and
offset: ' + offset );
};
\ No newline at end of file
diff --git a/modules/ve/ce/ve.ce.Surface.js b/modules/ve/ce/ve.ce.Surface.js
index 262d7ae..e6355fb 100644
--- a/modules/ve/ce/ve.ce.Surface.js
+++ b/modules/ve/ce/ve.ce.Surface.js
@@ -118,6 +118,21 @@
'g'
);
+/**
+ * Get the coordinates of the selection anchor.
+ *
+ * @method
+ * @static
+ */
+ve.ce.Surface.getSelectionRect = function () {
+ var rangySel = rangy.getSelection();
+ return {
+ start: rangySel.getStartDocumentPos(),
+ end: rangySel.getEndDocumentPos()
+ };
+};
+
+
/* Methods */
/*! Initialization */
@@ -306,7 +321,10 @@
* @param {jQuery.Event} e Key press event
*/
ve.ce.Surface.prototype.onDocumentKeyPress = function ( e ) {
- if ( ve.ce.Surface.isShortcutKey( e ) || e.which === 13 || e.which ===
8 || e.which === 0 ) {
+ if ( ve.ce.isShortcutKey( e ) ||
+ e.which === ve.Keys.DOM_VK_RETURN ||
+ e.which === ve.Keys.DOM_VK_BACK_SPACE ||
+ e.which === ve.Keys.DOM_VK_UNDEFINED ) {
return;
}
this.handleInsertion();
@@ -701,7 +719,7 @@
* @method
*/
ve.ce.Surface.prototype.handleUpOrDownArrowKey = function ( e ) {
- var selection, rangySelection, rangyRange, range, $fakeElement, $slug;
+ var selection, rangySelection, rangyRange, range, $element;
if ( !$.browser.msie ) {
return;
}
@@ -711,25 +729,21 @@
// Perform programatic handling only for selection that is expanded and
backwards according to
// model data but not according to browser data.
if ( !selection.isCollapsed() && selection.isBackwards() &&
!rangySelection.isBackwards() ) {
- $slug = this.documentView.getSlugAtOffset( selection.to );
- if ( $slug ) {
- rangyRange = rangy.createRange();
- rangyRange.selectNode( $slug[0] );
- rangySelection.setSingleRange( rangyRange );
- } else {
- $fakeElement = $ ( '<span>' ).html( ' ' ).css( {
'width' : '0px', 'display' : 'none' } );
+ $element = this.documentView.getSlugAtOffset( selection.to );
+ if ( !$element ) {
+ $element = $( '<span>' ).html( ' ' ).css( { 'width' :
'0px', 'display' : 'none' } );
rangySelection.anchorNode.splitText(
rangySelection.anchorOffset );
rangySelection.anchorNode.parentNode.insertBefore(
- $fakeElement[0],
+ $element[0],
rangySelection.anchorNode.nextSibling
);
- rangyRange = rangy.createRange();
- rangyRange.selectNode( $fakeElement[0] );
- rangySelection.setSingleRange( rangyRange );
}
+ rangyRange = rangy.createRange();
+ rangyRange.selectNode( $element[0] );
+ rangySelection.setSingleRange( rangyRange );
setTimeout( ve.bind( function() {
- if ( $fakeElement ) {
- $fakeElement.remove();
+ if ( !$element.hasClass( 've-ce-slug' ) ) {
+ $element.remove();
}
this.surfaceObserver.start();
this.surfaceObserver.stop( false );
@@ -940,23 +954,10 @@
* @param {boolean} backspace Key was a backspace
*/
ve.ce.Surface.prototype.handleDelete = function ( e, backspace ) {
- var sourceOffset,
- targetOffset,
- sourceSplitableNode,
- targetSplitableNode,
- tx,
- cursorAt,
- sourceNode,
- targetNode,
- sourceData,
- nodeToDelete,
- adjacentData,
- adjacentText,
- adjacentTextAfterMatch,
- endOffset,
- i,
- selection = this.model.getSelection(),
- containsInlineElements = false;
+ var sourceOffset, targetOffset, sourceSplitableNode,
targetSplitableNode, tx, cursorAt,
+ sourceNode, targetNode, sourceData, nodeToDelete, adjacentData,
adjacentText,
+ adjacentTextAfterMatch, endOffset, i, containsInlineElements =
false,
+ selection = this.model.getSelection();
if ( selection.isCollapsed() ) {
// Set source and target linmod offsets
@@ -1110,14 +1111,14 @@
);
if ( !range.isCollapsed() ) {
- start = this.getNodeAndOffset( range.start );
- end = this.getNodeAndOffset( range.end );
+ start = this.documentView.getNodeAndOffset( range.start );
+ end = this.documentView.getNodeAndOffset( range.end );
rangyRange.setStart( start.node, start.offset );
rangyRange.setEnd( end.node, end.offset );
rangySel.removeAllRanges();
rangySel.addRange( rangyRange, range.start !== range.from );
} else {
- start = this.getNodeAndOffset( range.start );
+ start = this.documentView.getNodeAndOffset( range.start );
rangyRange.setStart( start.node, start.offset );
rangySel.setSingleRange( rangyRange );
}
@@ -1192,90 +1193,6 @@
};
/**
- * Get a DOM node and DOM element offset for a document offset.
- *
- * The results of this function are meant to be used with rangy.
- *
- * @method
- * @param {number} offset Linear model offset
- * @returns {Object} Object containing a node and offset property where node
is an HTML element and
- * offset is the position within the element
- * @throws {Error} Offset could not be translated to a DOM element and offset
- */
-ve.ce.Surface.prototype.getNodeAndOffset = function ( offset ) {
- var node, startOffset, current, stack, item, $item, length,
- slug = this.documentView.getSlugAtOffset( offset );
- if ( slug ) {
- return { node: slug[0].childNodes[0], offset: 0 };
- }
- node = this.documentView.getNodeFromOffset( offset );
- startOffset = this.documentView.getDocumentNode().getOffsetFromNode(
node ) +
- ( ( node.isWrapped() ) ? 1 : 0 );
- current = [node.$.contents(), 0];
- stack = [current];
-
- while ( stack.length > 0 ) {
- if ( current[1] >= current[0].length ) {
- stack.pop();
- current = stack[ stack.length - 1 ];
- continue;
- }
- item = current[0][current[1]];
- if ( item.nodeType === Node.TEXT_NODE ) {
- length = item.textContent.length;
- if ( offset >= startOffset && offset <= startOffset +
length ) {
- return {
- node: item,
- offset: offset - startOffset
- };
- } else {
- startOffset += length;
- }
- } else if ( item.nodeType === Node.ELEMENT_NODE ) {
- $item = current[0].eq( current[1] );
- if ( $item.hasClass('ve-ce-slug') ) {
- if ( offset === startOffset ) {
- return {
- node: $item[0],
- offset: 1
- };
- }
- } else if ( $item.is( '.ve-ce-branchNode,
.ve-ce-leafNode' ) ) {
- length = $item.data( 'node'
).model.getOuterLength();
- if ( offset >= startOffset && offset <
startOffset + length ) {
- stack.push( [$item.contents(), 0] );
- current[1]++;
- current = stack[stack.length-1];
- continue;
- } else {
- startOffset += length;
- }
- } else {
- stack.push( [$item.contents(), 0] );
- current[1]++;
- current = stack[stack.length-1];
- continue;
- }
- }
- current[1]++;
- }
- throw new Error( 'Offset could not be translated to a DOM element and
offset: ' + offset );
-};
-
-/**
- * Check if keyboard shortcut modifier key is pressed.
- *
- * @method
- * @param {jQuery.Event} e Key press event
- */
-ve.ce.Surface.isShortcutKey = function ( e ) {
- if ( e.ctrlKey || e.metaKey ) {
- return true;
- }
- return false;
-};
-
-/**
* Get the number of consecutive clicks the user has performed.
*
* This is required for supporting double, tripple, etc. clicking across all
browsers.
@@ -1322,19 +1239,6 @@
};
/*! Getters */
-
-/**
- * Get the coordinates of the selection anchor.
- *
- * @method
- */
-ve.ce.Surface.prototype.getSelectionRect = function () {
- var rangySel = rangy.getSelection();
- return {
- start: rangySel.getStartDocumentPos(),
- end: rangySel.getEndDocumentPos()
- };
-};
/**
* Get the surface model.
diff --git a/modules/ve/ce/ve.ce.SurfaceObserver.js
b/modules/ve/ce/ve.ce.SurfaceObserver.js
index dd84229..79d9d31 100644
--- a/modules/ve/ce/ve.ce.SurfaceObserver.js
+++ b/modules/ve/ce/ve.ce.SurfaceObserver.js
@@ -117,6 +117,9 @@
*
* TODO: fixing selection in certain cases, handling selection across multiple
nodes in Firefox
*
+ * FIXME: Does not work well (selectionChange is not emited) when cursor is
placed inside a slug
+ * with a mouse.
+ *
* @method
* @param {boolean} async Poll asynchronously
* @emits contentChange
diff --git a/modules/ve/ce/ve.ce.js b/modules/ve/ce/ve.ce.js
index ab6644c..7973295 100644
--- a/modules/ve/ce/ve.ce.js
+++ b/modules/ve/ce/ve.ce.js
@@ -258,4 +258,14 @@
ve.ce.isArrowKey = function ( keyCode ) {
return ve.ce.isLeftOrRightArrowKey( keyCode ) ||
ve.ce.isUpOrDownArrowKey( keyCode );
+};
+
+/**
+ * Check if keyboard shortcut modifier key is pressed.
+ *
+ * @method
+ * @param {jQuery.Event} e Key press event
+ */
+ve.ce.isShortcutKey = function ( e ) {
+ return e.ctrlKey || e.metaKey;
};
\ No newline at end of file
diff --git a/modules/ve/ui/ve.ui.Context.js b/modules/ve/ui/ve.ui.Context.js
index 6a03782..492baaf 100644
--- a/modules/ve/ui/ve.ui.Context.js
+++ b/modules/ve/ui/ve.ui.Context.js
@@ -228,7 +228,7 @@
inspector = this.inspectors.getCurrent();
// Get cursor position
- position = this.surface.getView().getSelectionRect().end;
+ position = ve.ce.Surface.getSelectionRect().end;
if ( position ) {
// Get additional dimensions
$container = inspector ? this.inspectors.$ : this.$menu;
diff --git a/modules/ve/ve.js b/modules/ve/ve.js
index a71eb13..2b3a47a 100644
--- a/modules/ve/ve.js
+++ b/modules/ve/ve.js
@@ -883,6 +883,7 @@
// Based on the KeyEvent DOM Level 3 (add more as you need them)
//
http://www.w3.org/TR/2001/WD-DOM-Level-3-Events-20010410/DOM3-Events.html#events-Events-KeyEvent
ve.Keys = window.KeyEvent || {
+ 'DOM_VK_UNDEFINED': 0,
'DOM_VK_BACK_SPACE': 8,
'DOM_VK_RETURN': 13,
'DOM_VK_LEFT': 37,
--
To view, visit https://gerrit.wikimedia.org/r/55851
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic00221fa463205d04c9b52150c0dd15904493b1e
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Inez <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Christian <[email protected]>
Gerrit-Reviewer: Robmoen <[email protected]>
Gerrit-Reviewer: Trevor Parscal <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
