Merge "Prevent the creation of invalid MWInternalLinks"

VisualEditor.i18n.php

@@ -128,6 +128,7 @@ $messages['en'] = array(
 	'visualeditor-indentationbutton-outdent-tooltip' => 'Decrease indentation',
 	'visualeditor-inspector-close-tooltip' => 'Close',
 	'visualeditor-inspector-remove-tooltip' => 'Remove',
+	'visualeditor-linkinspector-illegal-title' => 'Invalid page title',
 	'visualeditor-linkinspector-suggest-external-link' => 'External link',
 	'visualeditor-linkinspector-suggest-matching-page' => 'Matching page',
 	'visualeditor-linkinspector-suggest-new-page' => 'New page',
@@ -425,6 +426,7 @@ Parameters:
 	'visualeditor-indentationbutton-outdent-tooltip' => 'Tooltip text for list outdent button',
 	'visualeditor-inspector-close-tooltip' => '{{Identical|Close}}',
 	'visualeditor-inspector-remove-tooltip' => 'Tooltip text for the button to remove an annotation in an inspector',
+	'visualeditor-linkinspector-illegal-title' => 'Warning that the entered text is not a valid page title.',
 	'visualeditor-linkinspector-suggest-external-link' => 'Label for an external (Web) link in the link inspector.
 {{Identical|External link}}',
 	'visualeditor-linkinspector-suggest-matching-page' => 'Label for suggested matching local wiki page or pages in the link inspector',

VisualEditor.php

@@ -659,6 +659,7 @@ $wgResourceModules += array(
 			'visualeditor-indentationbutton-outdent-tooltip',
 			'visualeditor-inspector-close-tooltip',
 			'visualeditor-inspector-remove-tooltip',
+			'visualeditor-linkinspector-illegal-title',
 			'visualeditor-linkinspector-suggest-external-link',
 			'visualeditor-linkinspector-suggest-matching-page',
 			'visualeditor-linkinspector-suggest-new-page',

modules/ve-mw/ui/inspectors/ve.ui.MWLinkInspector.js

@@ -43,7 +43,7 @@ ve.ui.MWLinkInspector.static.linkTargetInputWidget = ve.ui.MWLinkTargetInputWidg
  *
  * @method
  * @param {string} target Link target
- * @returns {ve.dm.MWInternalLinkAnnotation|ve.dm.MWExternalLinkAnnotation}
+ * @returns {ve.dm.MWInternalLinkAnnotation|ve.dm.MWExternalLinkAnnotation|null}
  */
 ve.ui.MWLinkInspector.prototype.getAnnotationFromText = function ( target ) {
 	var title;
@@ -57,7 +57,7 @@ ve.ui.MWLinkInspector.prototype.getAnnotationFromText = function ( target ) {
 				'href': target
 			}
 		} );
-	} else {
+	} else if ( ve.ui.MWLinkInspector.static.legalTitle.test( target ) ) {
 		// Internal link
 		// TODO: In the longer term we'll want to have autocompletion and existence and validity
 		// checks using AJAX
@@ -77,9 +77,20 @@ ve.ui.MWLinkInspector.prototype.getAnnotationFromText = function ( target ) {
 				'normalizedTitle': ve.dm.MWInternalLinkAnnotation.static.normalizeTitle( target )
 			}
 		} );
+	} else {
+		return null;
 	}
 };
 
+/* Static Properties */
+
+/**
+ * Regular expression matching a valid internal link
+ *
+ * @type {RegExp}
+ */
+ve.ui.MWLinkInspector.static.legalTitle = /^[ %!"$&'()*,\-.\/0-9:;=?@A-Z\\^_`a-z~\u0080-\u00FF+]+$/;
+
 /* Registration */
 
 ve.ui.inspectorFactory.register( 'mwLink', ve.ui.MWLinkInspector );

modules/ve-mw/ui/widgets/ve.ui.MWLinkTargetInputWidget.js

@@ -46,12 +46,19 @@ ve.mixinClass( ve.ui.MWLinkTargetInputWidget, ve.ui.LookupInputWidget );
 /**
  * Handle menu item select event.
  *
+ * If no item is selected then the input must be invalid, so clear the annotation.
+ * We shouldn't just leave the previous annotation as the user has no way of knowing
+ * what that might be. For example if "Foo{}Bar" is typed, this.annotation will be
+ * a link to "Foo".
+ *
  * @method
- * @param {ve.ui.MenuItemWidget} item Selected item
+ * @param {ve.ui.MenuItemWidget|null} item Selected item
  */
 ve.ui.MWLinkTargetInputWidget.prototype.onLookupMenuItemSelect = function ( item ) {
 	if ( item ) {
 		this.setAnnotation( item.getData() );
+	} else if ( this.annotation ) {
+		this.annotation = null;
 	}
 };
 
@@ -92,7 +99,7 @@ ve.ui.MWLinkTargetInputWidget.prototype.getLookupCacheItemFromData = function (
  * @returns {ve.ui.MenuItemWidget[]} Menu items
  */
 ve.ui.MWLinkTargetInputWidget.prototype.getLookupMenuItemsFromData = function ( data ) {
-	var i, len,
+	var i, len, item,
 		menu$$ = this.lookupMenu.$$,
 		items = [],
 		matchingPages = data,
@@ -116,14 +123,23 @@ ve.ui.MWLinkTargetInputWidget.prototype.getLookupMenuItemsFromData = function (
 
 	// Internal link
 	if ( !pageExists ) {
-		items.push( new ve.ui.MenuSectionItemWidget(
-			'newPage',
-			{ '$$': menu$$, 'label': ve.msg( 'visualeditor-linkinspector-suggest-new-page' ) }
-		) );
-		items.push( new ve.ui.MenuItemWidget(
-			this.getInternalLinkAnnotationFromTitle( this.value ),
-			{ '$$': menu$$, 'rel': 'newPage', 'label': this.value }
-		) );
+		if ( ve.ui.MWLinkInspector.static.legalTitle.test( this.value ) ) {
+			items.push( new ve.ui.MenuSectionItemWidget(
+				'newPage',
+				{ '$$': menu$$, 'label': ve.msg( 'visualeditor-linkinspector-suggest-new-page' ) }
+			) );
+			items.push( new ve.ui.MenuItemWidget(
+				this.getInternalLinkAnnotationFromTitle( this.value ),
+				{ '$$': menu$$, 'rel': 'newPage', 'label': this.value }
+			) );
+		} else {
+			item = new ve.ui.MenuSectionItemWidget(
+				'illegalTitle',
+				{ '$$': menu$$, 'label': ve.msg( 'visualeditor-linkinspector-illegal-title' ) }
+			);
+			item.$.addClass( 've-ui-mwLinkTargetInputWidget-warning' );
+			items.push( item );
+		}
 	}
 
 	// Matching pages

modules/ve/ui/inspectors/ve.ui.AnnotationInspector.js

@@ -23,6 +23,7 @@ ve.ui.AnnotationInspector = function VeUiAnnotationInspector( surface, config )
 	// Properties
 	this.initialAnnotation = null;
 	this.initialAnnotationHash = null;
+	this.initialText = null;
 	this.isNewAnnotation = false;
 };
 
@@ -75,8 +76,11 @@ ve.ui.AnnotationInspector.prototype.onSetup = function () {
 			// Create annotation from selection
 			truncatedFragment = fragment.truncateRange( 255 );
 			fragment = truncatedFragment;
-			annotation = this.getAnnotationFromText( fragment.getText() );
-			fragment.annotateContent( 'set', annotation );
+			this.initialText = fragment.getText();
+			annotation = this.getAnnotationFromText( this.initialText );
+			if ( annotation ) {
+				fragment.annotateContent( 'set', annotation );
+			}
 			this.isNewAnnotation = true;
 		}
 	} else {
@@ -129,7 +133,7 @@ ve.ui.AnnotationInspector.prototype.onClose = function ( action ) {
 
 	if ( remove ) {
 		clear = true;
-	} else {
+	} else if ( annotation ) {
 		if ( this.initialSelection.isCollapsed() ) {
 			insert = true;
 		}

modules/ve/ui/inspectors/ve.ui.LinkInspector.js

@@ -73,7 +73,13 @@ ve.ui.LinkInspector.prototype.onOpen = function () {
 		// Note: Focus input prior to setting target annotation
 		this.targetInput.$input.focus();
 		// Setup annotation
-		this.targetInput.setAnnotation( this.initialAnnotation );
+		if ( this.initialAnnotation ) {
+			this.targetInput.setAnnotation( this.initialAnnotation );
+		} else {
+			// If an initial annotation couldn't be created (e.g. the text was invalid),
+			// just populate the text we tried to create the annotation from
+			this.targetInput.setValue( this.initialText );
+		}
 		this.targetInput.$input.select();
 		this.surface.enable();
 	}, this ), 200 );