r63455 MediaWiki - Code Review archive

+			var t = context.fn.getOffset();

getOffset() takes a parameter; presumably this acts like getOffset(0), but I'm not even sure of that and I wrote that function. Instead of just doing this check for the first textnode in the document, why not do a general check for textnodes that are direct children of the body, something along the lines of:

if ( context.$content.children().length < context.$content.get( 0 ).childNodes.length ) { // textnodes count on the RHS but not on the LHS
	var $textnodes = $( context.$content.get( 0 ).childNodes ).not( '*' ); // textnodes don't match any selector, not even *
	$textnodes.each( function() { ..... } );
}

+				context.fn.refreshOffsets();

Please use purgeOffsets() instead, it purges the offset cache so offsets are regenerated the next time they're needed rather than right now. This matters in browsers whose setSelection() implementation doesn't use the offsets system (e.g. IE), and matters generally if you're not calling an offset-reliant function right after.

+				var cursorPos = context.fn.getCaretPosition()[0];
...
+				context.fn.setSelection( { start: cursorPos, end: cursorPos } );

This force-collapses the selection to the start. Is there a good reason to do this? If not, you should use start: cursorPos[0], end: cursorPos[1] and drop the [0] from the cursorPos assignment.

-					if ( sc.nodeName == '#text' && start >= sc.nodeValue.length ) {
+					if ( sc.nodeName == '#text' && start > sc.nodeValue.length ) {

This allows the start offset to be equal to the length. That's already out of bounds, the highest valid offset is length - 1, right?

The only issue I did not modify is the last one. From what I'm seeing I think length is a valid offset. Eg if length is 1, placing the cursor at 1 will place it after the first character.