jenkins-bot has submitted this change and it was merged.
Change subject: Prevent the creation of invalid MWInternalLinks
......................................................................
Prevent the creation of invalid MWInternalLinks
By testing against a regex of legal title characters we can determine
if the entered text is a valid internal link. If it isn't we should
prevent the link inpsector from creating/changing the annotation.
Bug: 33094
Change-Id: Ia1df602601e4e82fc351279e432c28c425f5157a
---
M VisualEditor.i18n.php
M VisualEditor.php
M modules/ve-mw/ui/inspectors/ve.ui.MWLinkInspector.js
M modules/ve-mw/ui/widgets/ve.ui.MWLinkTargetInputWidget.js
M modules/ve/ui/inspectors/ve.ui.AnnotationInspector.js
M modules/ve/ui/inspectors/ve.ui.LinkInspector.js
6 files changed, 56 insertions(+), 16 deletions(-)
Approvals:
Trevor Parscal: Looks good to me, approved
jenkins-bot: Verified
diff --git a/VisualEditor.i18n.php b/VisualEditor.i18n.php
index 9be6029..b164f29 100644
--- a/VisualEditor.i18n.php
+++ b/VisualEditor.i18n.php
@@ -110,6 +110,7 @@
'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',
@@ -383,6 +384,7 @@
'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',
diff --git a/VisualEditor.php b/VisualEditor.php
index b4db84c..703388e 100644
--- a/VisualEditor.php
+++ b/VisualEditor.php
@@ -652,6 +652,7 @@
'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',
diff --git a/modules/ve-mw/ui/inspectors/ve.ui.MWLinkInspector.js
b/modules/ve-mw/ui/inspectors/ve.ui.MWLinkInspector.js
index 945b347..05019cf 100644
--- a/modules/ve-mw/ui/inspectors/ve.ui.MWLinkInspector.js
+++ b/modules/ve-mw/ui/inspectors/ve.ui.MWLinkInspector.js
@@ -43,7 +43,7 @@
*
* @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 @@
'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 @@
'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 );
diff --git a/modules/ve-mw/ui/widgets/ve.ui.MWLinkTargetInputWidget.js
b/modules/ve-mw/ui/widgets/ve.ui.MWLinkTargetInputWidget.js
index cd42942..ba639e8 100644
--- a/modules/ve-mw/ui/widgets/ve.ui.MWLinkTargetInputWidget.js
+++ b/modules/ve-mw/ui/widgets/ve.ui.MWLinkTargetInputWidget.js
@@ -46,12 +46,19 @@
/**
* 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 @@
* @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 @@
// 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
diff --git a/modules/ve/ui/inspectors/ve.ui.AnnotationInspector.js
b/modules/ve/ui/inspectors/ve.ui.AnnotationInspector.js
index c5f4c9c..3cc8de5 100644
--- a/modules/ve/ui/inspectors/ve.ui.AnnotationInspector.js
+++ b/modules/ve/ui/inspectors/ve.ui.AnnotationInspector.js
@@ -23,6 +23,7 @@
// Properties
this.initialAnnotation = null;
this.initialAnnotationHash = null;
+ this.initialText = null;
this.isNewAnnotation = false;
};
@@ -75,8 +76,11 @@
// 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 @@
if ( remove ) {
clear = true;
- } else {
+ } else if ( annotation ) {
if ( this.initialSelection.isCollapsed() ) {
insert = true;
}
diff --git a/modules/ve/ui/inspectors/ve.ui.LinkInspector.js
b/modules/ve/ui/inspectors/ve.ui.LinkInspector.js
index 6c8298c..930d4b4 100644
--- a/modules/ve/ui/inspectors/ve.ui.LinkInspector.js
+++ b/modules/ve/ui/inspectors/ve.ui.LinkInspector.js
@@ -73,7 +73,13 @@
// 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 );
--
To view, visit https://gerrit.wikimedia.org/r/77099
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia1df602601e4e82fc351279e432c28c425f5157a
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Esanders <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: Siebrand <[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
