jenkins-bot has submitted this change and it was merged.
Change subject: mw.ViewPageTarget: Show save errors in save dialog instead of
alert
......................................................................
mw.ViewPageTarget: Show save errors in save dialog instead of alert
To trigger a save rejection from the api, set:
$wgSpamRegex = '/spam/i';
and making an edit adding the word "spam" to a page.
Class changes:
* Rename message system in the save dialog from "warning" to
"message" as it will now contain both warnings and errors.
(css class, class property, method names, ..)
Localisation:
* Remove ugly hardcoded and wikitext-requiring "'''Warning:'''"
prefix from the warning message, instead have a message for the
word "Warning" and re-use this in #showMessage for each message
of type "warning" (bolding applied in code instead of in i18n).
* Rename visualeditor-savedialog-dirtywarning to
visualeditor-savedialog-warning-dirty and remove from
VisualEditorMessagesModule.php as it no longer requires pre-
processing from the server.
Clean up:
* Re-alphabetise the order of some messages.
* Clean up duplication and redundant logic in mw.Target#onSaveError
and mw.ViewPageTarget#onSaveError.
Bug: 50350
Change-Id: I3daf631fb0d62ba88e05aa50c77c9940d61395a0
---
M VisualEditor.i18n.php
M VisualEditor.php
M VisualEditorMessagesModule.php
M modules/ve/init/mw/styles/ve.init.mw.ViewPageTarget.css
M modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js
M modules/ve/init/mw/ve.init.mw.Target.js
6 files changed, 104 insertions(+), 59 deletions(-)
Approvals:
Catrope: Looks good to me, approved
jenkins-bot: Verified
diff --git a/VisualEditor.i18n.php b/VisualEditor.i18n.php
index 59b3917..b6f3501 100644
--- a/VisualEditor.i18n.php
+++ b/VisualEditor.i18n.php
@@ -123,23 +123,25 @@
'visualeditor-outline-control-move-down' => 'Move item down',
'visualeditor-outline-control-move-up' => 'Move item up',
'visualeditor-preference-enable' => 'Enable VisualEditor (only in the
[[{{MediaWiki:Visualeditor-mainnamespacepagelink}}|main]] and
[[{{MediaWiki:Visualeditor-usernamespacepagelink}}|user]] namespaces)',
- 'visualeditor-referencelist-isempty' => 'There are no references with
the group "$1" on this page.',
- 'visualeditor-referencelist-missingref' => 'This reference is defined
in a template or other generated block, and cannot yet be edited with
VisualEditor.',
'visualeditor-reference-input-placeholder' => 'What do you want to
reference?',
'visualeditor-reference-search-create' => 'Create new source',
'visualeditor-reference-search-reuse' => 'Use an existing source',
- 'visualeditor-savedialog-dirtywarning' => "'''Warning:''' Your edit may
have been corrupted – please review before saving.",
+ 'visualeditor-referencelist-isempty' => 'There are no references with
the group "$1" on this page.',
+ 'visualeditor-referencelist-missingref' => 'This reference is defined
in a template or other generated block, and cannot yet be edited with
VisualEditor.',
'visualeditor-savedialog-label-create' => 'Create page',
+ 'visualeditor-savedialog-label-error' => 'Error',
'visualeditor-savedialog-label-report' => 'Report problem',
'visualeditor-savedialog-label-resolve-conflict' => 'Resolve conflict',
'visualeditor-savedialog-label-restore' => 'Restore page',
'visualeditor-savedialog-label-review' => 'Review your changes',
'visualeditor-savedialog-label-review-good' => 'Return to save form',
'visualeditor-savedialog-label-save' => 'Save page',
+ 'visualeditor-savedialog-label-warning' => 'Warning',
'visualeditor-savedialog-title-conflict' => 'Conflict',
'visualeditor-savedialog-title-nochanges' => 'No changes',
'visualeditor-savedialog-title-review' => 'Review your changes',
'visualeditor-savedialog-title-save' => 'Save your changes',
+ 'visualeditor-savedialog-warning-dirty' => 'Your edit may have been
corrupted – please review before saving.',
'visualeditor-saveerror' => 'Error saving data to server: $1.',
'visualeditor-serializeerror' => 'Error loading data from server: $1.',
'visualeditor-toolbar-cancel' => 'Cancel',
@@ -379,6 +381,7 @@
See also:
* {{msg-mw|Visualeditor-reference-search-create}}',
'visualeditor-savedialog-label-create' => 'Label text for save button
when the user is creating a new page',
+ 'visualeditor-savedialog-label-error' => 'Label in front of a save
dialog error sentence, separated by {{msg-mw|colon-separator}}.',
'visualeditor-savedialog-label-report' => 'Label for button to trigger
report',
'visualeditor-savedialog-label-resolve-conflict' => 'Label for button
to start resoliving an edit conflict',
'visualeditor-savedialog-label-restore' => 'Label text for save button
when the user is editing a previous revision',
@@ -386,6 +389,7 @@
'visualeditor-savedialog-label-review-good' => 'Label for button to go
back to the save form',
'visualeditor-savedialog-label-save' => 'Label text for save button
when the user is editing a current revision of an extant page.
{{Identical|Save page}}',
+ 'visualeditor-savedialog-label-warning' => 'Label in front of a save
dialog warning sentence, separated by {{msg-mw|colon-separator}}.',
'visualeditor-savedialog-title-conflict' => 'Title for save dialog
slide if there is an edit conflict',
'visualeditor-savedialog-title-nochanges' => 'Title for save dialog
slide for the wikitext diff if there are no changes',
'visualeditor-savedialog-title-review' => 'Title for save dialog slide
for the wikitext diff',
diff --git a/VisualEditor.php b/VisualEditor.php
index e044e2a..5641dfa 100644
--- a/VisualEditor.php
+++ b/VisualEditor.php
@@ -608,22 +608,25 @@
'visualeditor-outline-control-move-down',
'visualeditor-outline-control-move-up',
'visualeditor-outline-control-move-up',
- 'visualeditor-referencelist-isempty',
- 'visualeditor-referencelist-missingref',
'visualeditor-reference-input-placeholder',
'visualeditor-reference-search-create',
'visualeditor-reference-search-reuse',
+ 'visualeditor-referencelist-isempty',
+ 'visualeditor-referencelist-missingref',
'visualeditor-savedialog-label-create',
+ 'visualeditor-savedialog-label-error',
'visualeditor-savedialog-label-report',
'visualeditor-savedialog-label-resolve-conflict',
'visualeditor-savedialog-label-restore',
'visualeditor-savedialog-label-review',
'visualeditor-savedialog-label-review-good',
'visualeditor-savedialog-label-save',
+ 'visualeditor-savedialog-label-warning',
'visualeditor-savedialog-title-conflict',
'visualeditor-savedialog-title-nochanges',
'visualeditor-savedialog-title-review',
'visualeditor-savedialog-title-save',
+ 'visualeditor-savedialog-warning-dirty',
'visualeditor-saveerror',
'visualeditor-serializeerror',
'visualeditor-toolbar-cancel',
diff --git a/VisualEditorMessagesModule.php b/VisualEditorMessagesModule.php
index 71c16b0..f7a4743 100644
--- a/VisualEditorMessagesModule.php
+++ b/VisualEditorMessagesModule.php
@@ -46,7 +46,6 @@
'visualeditor-browserwarning' => array(
'visualeditor-browserwarning' ),
'visualeditor-report-notice' => array(
'visualeditor-report-notice' ),
'missingsummary' => array( 'missingsummary' ),
- 'visualeditor-savedialog-dirtywarning' => array(
'visualeditor-savedialog-dirtywarning' ),
);
// Override message value
diff --git a/modules/ve/init/mw/styles/ve.init.mw.ViewPageTarget.css
b/modules/ve/init/mw/styles/ve.init.mw.ViewPageTarget.css
index 72fe6aa..783e10f 100644
--- a/modules/ve/init/mw/styles/ve.init.mw.ViewPageTarget.css
+++ b/modules/ve/init/mw/styles/ve.init.mw.ViewPageTarget.css
@@ -268,7 +268,7 @@
margin-bottom: 1em;
}
-.ve-init-mw-viewPageTarget-saveDialog-warnings,
+.ve-init-mw-viewPageTarget-saveDialog-messages,
.ve-init-mw-viewPageTarget-saveDialog-conflict,
.ve-init-mw-viewPageTarget-saveDialog-nochanges {
font-size: 0.8em;
diff --git a/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js
b/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js
index 3d6df5f..b78fffd 100644
--- a/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js
+++ b/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js
@@ -87,7 +87,7 @@
this.actFromPopState = false;
this.scrollTop = null;
this.currentUri = currentUri;
- this.warnings = {};
+ this.messages = {};
this.restoring = this.oldid !== mw.config.get( 'wgCurRevisionId' );
this.section = currentUri.query.vesection || null;
this.namespaceName = mw.config.get( 'wgCanonicalNamespace' );
@@ -245,7 +245,7 @@
for="ve-init-mw-viewPageTarget-saveDialog-watchList"></label>\
<label
class="ve-init-mw-viewPageTarget-saveDialog-editSummaryCount"></label>\
</div>\
- <div
class="ve-init-mw-viewPageTarget-saveDialog-warnings"></div>\
+ <div
class="ve-init-mw-viewPageTarget-saveDialog-messages"></div>\
<div
class="ve-init-mw-viewPageTarget-saveDialog-actions">\
<div
class="ve-init-mw-viewPageTarget-saveDialog-dirtymsg"></div>\
<div
class="ve-init-mw-viewPageTarget-saveDialog-working"></div>\
@@ -466,25 +466,41 @@
* @param {Object|null} data API response data
*/
ve.init.mw.ViewPageTarget.prototype.onSaveError = function ( jqXHR, status,
data ) {
+ var editApi;
this.saveDialogSaveButton.setDisabled( false );
this.$saveDialogLoadingIcon.hide();
- this.clearWarning( 'captcha' );
+ this.clearMessage( 'api-save-error' );
- // Captcha "errors" usually aren't errors. We simply don't know about
them ahead
- // of time, so we save once, then (if required) we get a captcha back
and try again
- // with captcha.
- // TODO: ConfirmEdit API is horrible, there is no reliable way to know
whether
- // it is a "math", "question" or "fancy" type of captcha. They all
expose differently
- // named properties in the API for different things. At this point we
only support
- // the FancyCaptha which we very intuitively detect by the presence of
a "url" property.
- if ( data.edit && data.edit.captcha && data.edit.captcha.url ) {
+ // Handle empty response
+ if ( !data ) {
+ this.showMessage(
+ 'api-save-error',
+ ve.msg( 'visualeditor-saveerror', 'Empty server
response' ),
+ {
+ wrap: 'error'
+ }
+ );
+ this.saveDialogSaveButton.setDisabled( true );
+ return;
+ }
+
+ // Handle captcha
+ // Captcha "errors" usually aren't errors. We simply don't know about
them ahead of time,
+ // so we save once, then (if required) we get an error with a captcha
back and try again after
+ // the user solved the captcha.
+ // TODO: ConfirmEdit API is horrible, there is no reliable way to know
whether it is a "math",
+ // "question" or "fancy" type of captcha. They all expose differently
named properties in the
+ // API for different things in the UI. At this point we only support
the FancyCaptha which we
+ // very intuitively detect by the presence of a "url" property.
+ editApi = data && data.visualeditor && data.visualeditor.edit;
+ if ( editApi && editApi.captcha && editApi.captcha.url ) {
this.captcha = {
input: new ve.ui.TextInputWidget(),
- id: data.edit.captcha.id
+ id: editApi.captcha.id
};
- this.showWarning(
- 'captcha',
+ this.showMessage(
+ 'api-save-error',
$( '<div>').append(
// msg: simplecaptcha-edit, fancycaptcha-edit,
..
$( '<p>' ).append(
@@ -493,7 +509,7 @@
$( $.parseHTML( mw.message(
'fancycaptcha-edit' ).parse() ) )
.filter( 'a' ).attr( 'target',
'_blank ' ).end()
),
- $( '<img>' ).attr( 'src', data.edit.captcha.url
),
+ $( '<img>' ).attr( 'src', editApi.captcha.url ),
this.captcha.input.$
),
{
@@ -503,8 +519,15 @@
return;
}
- // TODO: Don't use alert.
- alert( ve.msg( 'visualeditor-saveerror', status ) );
+ // Handle (other) unknown and/or unrecoverable errors
+ this.showMessage(
+ 'api-save-error',
+ document.createTextNode( data.error && ( data.error.info ||
data.error.code ) || 'Invalid error code' ),
+ {
+ wrap: 'error'
+ }
+ );
+ this.saveDialogSaveButton.setDisabled( true );
};
/**
@@ -776,16 +799,21 @@
// reset save start and any old captcha data
this.saveStart = +new Date();
if ( this.captcha ) {
- this.clearWarning( 'captcha' );
+ this.clearMessage( 'captcha' );
delete this.captcha;
}
if (
+mw.user.options.get( 'forceeditsummary' ) &&
saveOptions.summary === '' &&
- !this.warnings.missingsummary
+ !this.messages.missingsummary
) {
- this.showWarning( 'missingsummary',
ve.init.platform.getParsedMessage( 'missingsummary' ) );
+ this.showMessage(
+ 'missingsummary',
+ // Wrap manually since this core message already
includes a bold "Warning:" label
+ $( '<p>' ).append( ve.init.platform.getParsedMessage(
'missingsummary' ) ),
+ { wrap: false }
+ );
} else {
this.saveDialogSaveButton.setDisabled( true );
this.$saveDialogLoadingIcon.show();
@@ -1690,12 +1718,14 @@
.not( $slide )
.hide();
- // Old warnings should not persist after slide changes
- this.clearAllWarnings();
+ // Old messages should not persist after slide changes
+ this.clearAllMessages();
+ // Reset save button if we disabled it for e.g. unrecoverable spam error
+ this.saveDialogSaveButton.setDisabled( false );
if ( slide === 'save' ) {
if ( !this.sanityCheckVerified ) {
- this.showWarning( 'dirtywarning',
ve.init.platform.getParsedMessage( 'visualeditor-savedialog-dirtywarning' ) );
+ this.showMessage( 'dirtywarning', mw.msg(
'visualeditor-savedialog-warning-dirty' ) );
}
}
@@ -2117,48 +2147,56 @@
};
/**
- * Show an inline warning.
- * @param {string} name Warning's unique name
- * @param {string|jQuery} message Warning message (string of HTML, not text,
or jQuery object)
+ * Show a message in the save dialog.
+ *
+ * @param {string} name Message's unique name
+ * @param {string|jQuery} message Message content (string of HTML or jQuery
object)
* @param {Object} [options]
- * @param {boolean} [options.wrap=true] Wrap the message in a paragraph.
+ * @param {boolean} [options.wrap="warning"] Whether to wrap the message in a
paragraph and if
+ * so, how. One of "warning", "error" or false.
*/
-ve.init.mw.ViewPageTarget.prototype.showWarning = function ( name, message,
options ) {
- var $warning;
- if ( !this.warnings[name] ) {
+ve.init.mw.ViewPageTarget.prototype.showMessage = function ( name, message,
options ) {
+ var $message;
+ if ( !this.messages[name] ) {
options = options || {};
- $warning = $( '<div
class="ve-init-mw-viewPageTarget-saveDialog-warning"></div>' );
+ $message = $( '<div
class="ve-init-mw-viewPageTarget-saveDialog-message"></div>' );
if ( options.wrap !== false ) {
- $warning.append( $( '<p>').append( message ) );
+ $message.append( $( '<p>').append(
+ // visualeditor-savedialog-label-error
+ // visualeditor-savedialog-label-warning
+ $( '<strong>' ).text( mw.msg(
'visualeditor-savedialog-label-' + options.wrap ) ),
+ document.createTextNode( mw.msg(
'colon-separator' ) ),
+ message
+ ) );
} else {
- $warning.append( message );
+ $message.append( message );
}
- this.$saveDialog.find(
'.ve-init-mw-viewPageTarget-saveDialog-warnings' )
- .append( $warning );
+ this.$saveDialog.find(
'.ve-init-mw-viewPageTarget-saveDialog-messages' )
+ .append( $message );
- this.warnings[name] = $warning;
+ this.messages[name] = $message;
}
};
/**
- * Remove an inline warning.
- * @param {string} name Warning's unique name
+ * Remove a message from the save dialog.
+ * @param {string} name Message's unique name
*/
-ve.init.mw.ViewPageTarget.prototype.clearWarning = function ( name ) {
- if ( this.warnings[name] ) {
- this.warnings[name].remove();
- delete this.warnings[name];
+ve.init.mw.ViewPageTarget.prototype.clearMessage = function ( name ) {
+ if ( this.messages[name] ) {
+ this.messages[name].remove();
+ delete this.messages[name];
}
};
/**
- * Remove all inline warnings.
+ * Remove all messages from the save dialog.
*/
-ve.init.mw.ViewPageTarget.prototype.clearAllWarnings = function () {
+ve.init.mw.ViewPageTarget.prototype.clearAllMessages = function () {
this.$saveDialog
- .find( '.ve-init-mw-viewPageTarget-saveDialog-warnings' )
+ .find( '.ve-init-mw-viewPageTarget-saveDialog-messages' )
.empty();
- this.warnings = {};
+ this.messages = {};
};
/**
diff --git a/modules/ve/init/mw/ve.init.mw.Target.js
b/modules/ve/init/mw/ve.init.mw.Target.js
index 5735026..958c28a 100644
--- a/modules/ve/init/mw/ve.init.mw.Target.js
+++ b/modules/ve/init/mw/ve.init.mw.Target.js
@@ -313,21 +313,22 @@
this.saving = false;
var data = response.visualeditor;
if ( !data && !response.error ) {
- ve.init.mw.Target.onSaveError.call( this, null, 'Invalid
response from server', null );
+ ve.init.mw.Target.onSaveError.call( this, null, 'Invalid
response from server', response );
} else if ( response.error ) {
if ( response.error.code === 'editconflict' ) {
this.emit( 'editConflict' );
} else {
- ve.init.mw.Target.onSaveError.call(
- this, null, 'Unsuccessful request: ' +
response.error.info, null
- );
+ ve.init.mw.Target.onSaveError.call( this, null, 'Save
failure', response );
}
} else if ( data.result !== 'success' ) {
// Note, this could be any of db failure, hookabort, badtoken
or even a captcha
- ve.init.mw.Target.onSaveError.call( this, null, 'Save failure',
data );
+ ve.init.mw.Target.onSaveError.call( this, null, 'Save failure',
response );
} else if ( typeof data.content !== 'string' ) {
ve.init.mw.Target.onSaveError.call(
- this, null, 'Invalid HTML content in response from
server', null
+ this,
+ null,
+ 'Invalid HTML content in response from server',
+ response
);
} else {
mw.config.set( 'wgCurRevisionId', data.newrevid );
--
To view, visit https://gerrit.wikimedia.org/r/71293
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3daf631fb0d62ba88e05aa50c77c9940d61395a0
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Krinkle <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
