Mattflaschen has uploaded a new change for review.
https://gerrit.wikimedia.org/r/279567
Change subject: Remove unused error messages, switch API errors to HTML, and
handle that on the client.
......................................................................
Remove unused error messages, switch API errors to HTML, and handle that on the
client.
I tried to find ways to make the errors short text messages (which is why the
Flow messages
were converted to be such), rather than (sometimes-long) HTML. However, the
errors in core
and other extensions use wikitext rather than plaintext, and the mechanism to
handle this (messageMap)
completely lacks i18n (English-only).
So until the i18n error system is improved, I think we have to use HTML.
I also removed unused Flow errors.
Bug: T121137
Change-Id: I0ca448380491400400f9bf35f812b4f73f67d26f
---
M i18n/en.json
M i18n/qqq.json
M includes/Api/ApiFlowBase.php
M includes/Api/ApiFlowBaseGet.php
M includes/Api/ApiFlowBasePost.php
M includes/Block/Topic.php
M includes/Exception/ExceptionHandling.php
M includes/Model/Workflow.php
M includes/SubmissionHandler.php
M modules/flow/ui/widgets/mw.flow.ui.BoardDescriptionWidget.js
M modules/flow/ui/widgets/mw.flow.ui.EditPostWidget.js
M modules/flow/ui/widgets/mw.flow.ui.EditTopicSummaryWidget.js
M modules/flow/ui/widgets/mw.flow.ui.NewTopicWidget.js
M modules/flow/ui/widgets/mw.flow.ui.ReplyWidget.js
M modules/flow/ui/widgets/mw.flow.ui.TopicTitleWidget.js
M modules/mw.flow.Initializer.js
16 files changed, 56 insertions(+), 42 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow
refs/changes/67/279567/1
diff --git a/i18n/en.json b/i18n/en.json
index db04362..8e9c69c 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -131,22 +131,16 @@
"flow-error-allowcreation-already-exists": "Page already exists at
destination, so a Flow board can not be moved there.",
"flow-error-allowcreation-flow-create-board": "User does not have the
\"{{int:right-flow-create-board}}\" permission",
"flow-error-http": "An error occurred while contacting the server.",
- "flow-error-other": "An unexpected error occurred.",
- "flow-error-external": "An error occurred.<br />The error message
received was: $1",
+ "flow-error-external": "An error occurred. The error message received
was: $1",
"flow-error-topic-is-locked": "This topic is resolved, so further
activity is not possible.",
"flow-error-lock-moderated-post": "You cannot mark a moderated post as
resolved.",
- "flow-error-external-multi": "Errors were encountered.<br />$1",
"flow-error-missing-content": "Post has no content. Content is required
to save a post.",
"flow-error-missing-summary": "You must submit a summary.",
"flow-error-missing-title": "Topic has no title. Title is required to
save a topic.",
"flow-error-parsoid-failure": "Unable to transfer content: Error
contacting the server for conversion between wikitext and HTML. Please check
your Internet connection or try again later if the problem persists. If you
still get this error please file a bug",
"flow-error-missing-replyto": "No \"replyTo\" parameter was supplied.
This parameter is required for the \"reply\" action.",
- "flow-error-invalid-replyto": "\"replyTo\" parameter was invalid. The
specified post could not be found.",
- "flow-error-delete-failure": "Deletion of this item failed.",
- "flow-error-hide-failure": "Hiding this item failed.",
"flow-error-missing-postId": "No \"postId\" parameter was supplied.
This parameter is required to manipulate a post.",
"flow-error-invalid-postId": "\"postId\" parameter was invalid. The
specified post ($1) could not be found.",
- "flow-error-restore-failure": "Restoration of this item failed.",
"flow-error-invalid-moderation-state": "An invalid value for a
parameter ('moderationState') was submitted to the Flow API.",
"flow-error-invalid-moderation-reason": "Please provide a reason for
the moderation.",
"flow-error-not-allowed": "Insufficient permissions to execute this
action.",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index 720ac4c..9d30f86 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -141,22 +141,16 @@
"flow-error-allowcreation-already-exists": "Internal error when
attempting to enable Flow on a specific page, and $mustNotExist was set true,
but the page does already exist",
"flow-error-allowcreation-flow-create-board": "Internal error when
attempting to enable Flow on a specific page, and user does not have
flow-create-board",
"flow-error-http": "Used as error message on HTTP error.",
- "flow-error-other": "Used as generic error message.",
- "flow-error-external": "Uses as error message. Parameters:\n* $1 -
error message\nSee also:\n* {{msg-mw|Flow-error-external-multi}}",
+ "flow-error-external": "Used as error message. Parameters:\n* $1 -
error message",
"flow-error-topic-is-locked": "Used as error message when a user
attempts to moderate/create/edit title/post when a topic is resolved (locked).",
"flow-error-lock-moderated-post": "Used as error message when user
attempts to resolve (lock) a moderated topic/post.",
- "flow-error-external-multi": "Used as error message. Parameters:\n* $1
- list of error messages\nSee also:\n* {{msg-mw|Flow-error-external}}",
"flow-error-missing-content": "Used as error
message.\n{{Related|Flow-error-missing}}",
"flow-error-missing-summary": "Used as error message when a summary is
expected, but is not submitted. Zero-length summaries are
allowed.\n{{Related|Flow-error-missing}}",
"flow-error-missing-title": "Used as error
message.\n{{Related|Flow-error-missing}}",
"flow-error-parsoid-failure": "Used as error message.\n\nParsoid is a
bidirectional wikitext parser and runtime. Converts back and forth between
wikitext and HTML/XML DOM with RDFa. See [[mw:Parsoid]].",
"flow-error-missing-replyto": "Used as error message.\n\nThe variable
name \"replyTo\" is invisible to users, so \"replyTo\" can be
translated.\n{{related|Flow-error-missing}}",
- "flow-error-invalid-replyto": "Used as error message.\n\nThe variable
name \"replyTo\" is invisible to users, so \"replyTo\" can be translated.",
- "flow-error-delete-failure": "Used as error message.\n\n\"this item\"
refers either \"this topic\" or \"this post\".",
- "flow-error-hide-failure": "Used as error message.\n\n\"this item\"
refers either \"this topic\" or \"this post\".",
"flow-error-missing-postId": "Used as error message when
deleting/restoring a post.\n\n\"manipulate\" refers either \"delete\" or
\"restore\".\n{{related|Flow-error-missing}}",
"flow-error-invalid-postId": "Used as error message when
deleting/restoring a post.\n\nThe variable name \"postId\" is invisible to
users, so \"postId\" can be translated.\n\nParameters:\n* $1 - contains the
postId that was specified",
- "flow-error-restore-failure": "Used as error message when restoring a
post.\n\n\"this item\" seems to refer \"this post\".",
"flow-error-invalid-moderation-state": "Used as error
message.\n\nUsually indicates a code bug (possibly in a third-party use of the
Flow API), so technical terminology is okay.\n\nValid values for
moderationState are: (none), hidden, deleted, suppressed. Do not translate the
word 'moderationState'.",
"flow-error-invalid-moderation-reason": "Used as error message when no
reason is given for the moderation of a post.",
"flow-error-not-allowed": "Error message when the user has insufficient
permissions to execute this action",
diff --git a/includes/Api/ApiFlowBase.php b/includes/Api/ApiFlowBase.php
index 09e22b0..fd14801 100644
--- a/includes/Api/ApiFlowBase.php
+++ b/includes/Api/ApiFlowBase.php
@@ -111,6 +111,20 @@
if ( $block->hasErrors() ) {
$errors = $block->getErrors();
+ // API localization is not implemented fully
yet.
+ // See
https://www.mediawiki.org/wiki/API:Localisation#Errors_and_warnings and T37074
+ // We probably really want to use brief
messages with ->text() (like dieUsageMsg).
+ //
+ // But we can't use ->text() with all these
messages (though I changed
+ // the Flow messages to support this). Ones
provided by core or extensions are often
+ // long-form and use wikitext.
+ //
+ // The standard mechanism to deal with that is
dieUsageMsg, but that is English-only,
+ // so we can't use it until that's solved.
That means we have to use the long-form HTML
+ // rendering, and clients need to support that.
+ //
+ // Also, it would be nice to use dieBlocked to
provide detailed block information, but
+ // that is also English-only.
foreach( $errors as $key ) {
$this->dieUsage(
$block->getErrorMessage( $key
)->parse(),
diff --git a/includes/Api/ApiFlowBaseGet.php b/includes/Api/ApiFlowBaseGet.php
index 8b7651d..e67b9ed 100644
--- a/includes/Api/ApiFlowBaseGet.php
+++ b/includes/Api/ApiFlowBaseGet.php
@@ -42,7 +42,7 @@
// block should've been able to render a GET request)
if ( !$output[$action]['result'] ) {
$this->dieUsage(
- wfMessage( 'flow-error-no-render' )->parse(),
+ $this->msg( 'flow-error-no-render' )->text(),
'no-render',
200,
array()
diff --git a/includes/Api/ApiFlowBasePost.php b/includes/Api/ApiFlowBasePost.php
index 1b8dc7d..109f154 100644
--- a/includes/Api/ApiFlowBasePost.php
+++ b/includes/Api/ApiFlowBasePost.php
@@ -30,7 +30,7 @@
// block should've been able to process the POST request)
if ( !count( $blocksToCommit ) ) {
$this->dieUsage(
- wfMessage( 'flow-error-no-commit' )->parse(),
+ $this->msg( 'flow-error-no-commit' )->text(),
'no-commit',
200,
array()
diff --git a/includes/Block/Topic.php b/includes/Block/Topic.php
index 31c3e74..e30df0b 100644
--- a/includes/Block/Topic.php
+++ b/includes/Block/Topic.php
@@ -883,6 +883,8 @@
'lim' => 10,
'showIfEmpty' => false,
// i18n messages:
+ //
flow-error-not-allowed-hide-extract
+ //
flow-error-not-allowed-reply-to-hide-topic-extract
//
flow-error-not-allowed-delete-extract
//
flow-error-not-allowed-reply-to-delete-topic-extract
//
flow-error-not-allowed-suppress-extract
diff --git a/includes/Exception/ExceptionHandling.php
b/includes/Exception/ExceptionHandling.php
index 21f2478..9637557 100644
--- a/includes/Exception/ExceptionHandling.php
+++ b/includes/Exception/ExceptionHandling.php
@@ -57,6 +57,7 @@
* Error code list for this exception
*/
protected function getErrorCodeList() {
+ // flow-error-default
return array ( 'default' );
}
@@ -140,11 +141,12 @@
*/
class InvalidInputException extends FlowException {
protected function getErrorCodeList() {
+ // Comments are i18n messages, for grepping
return array (
- 'invalid-input',
- 'missing-revision',
- 'revision-comparison',
- 'invalid-workflow'
+ 'invalid-input', // flow-error-invalid-input
+ 'missing-revision', // flow-error-missing-revision
+ 'revision-comparison', // flow-error-revision-comparison
+ 'invalid-workflow', // flow-error-invalid-workflow
);
}
@@ -164,6 +166,7 @@
*/
class InvalidActionException extends FlowException {
protected function getErrorCodeList() {
+ // flow-error-invalid-action
return array ( 'invalid-action' );
}
@@ -205,6 +208,7 @@
*/
class FailCommitException extends FlowException {
protected function getErrorCodeList() {
+ // flow-error-fail-commit
return array ( 'fail-commit' );
}
}
@@ -214,6 +218,7 @@
*/
class PermissionException extends FlowException {
protected function getErrorCodeList() {
+ // flow-error-insufficient-permission
return array ( 'insufficient-permission' );
}
@@ -232,12 +237,12 @@
class InvalidDataException extends FlowException {
protected function getErrorCodeList() {
return array (
- 'invalid-title',
- 'fail-load-data',
- 'fail-load-history',
- 'fail-search',
- 'missing-topic-title',
- 'missing-metadata'
+ 'invalid-title', // flow-error-invalid-title
+ 'fail-load-data', // flow-error-fail-load-data
+ 'fail-load-history', // flow-error-fail-load-history
+ 'fail-search', // flow-error-fail-search
+ 'missing-topic-title', // flow-error-missing-topic-title
+ 'missing-metadata', // flow-error-missing-metadata
);
}
}
@@ -247,6 +252,7 @@
*/
class DataModelException extends FlowException {
protected function getErrorCodeList() {
+ // flow-error-process-data
return array ( 'process-data' );
}
}
@@ -256,6 +262,7 @@
*/
class DataPersistenceException extends FlowException {
protected function getErrorCodeList() {
+ // flow-error-process-data
return array ( 'process-data' );
}
}
@@ -265,6 +272,7 @@
*/
class NoParserException extends FlowException {
protected function getErrorCodeList() {
+ // flow-error-process-wikitext
return array ( 'process-wikitext' );
}
}
@@ -274,6 +282,7 @@
*/
class WikitextException extends FlowException {
protected function getErrorCodeList() {
+ // flow-error-process-wikitext
return array ( 'process-wikitext' );
}
}
@@ -283,6 +292,7 @@
*/
class NoIndexException extends FlowException {
protected function getErrorCodeList() {
+ // flow-error-no-index
return array ( 'no-index' );
}
}
@@ -317,6 +327,7 @@
*/
class UnknownWorkflowIdException extends InvalidInputException {
protected function getErrorCodeList() {
+ // flow-error-invalid-input
return array( 'invalid-input' );
}
@@ -335,6 +346,7 @@
*/
class InvalidTopicUuidException extends InvalidInputException {
protected function getErrorCodeList() {
+ // flow-error-invalid-input
return array( 'invalid-input' );
}
@@ -352,6 +364,7 @@
*/
class InvalidUndeleteException extends InvalidActionException {
protected function getErrorCodeList() {
+ // flow-error-invalid-undelete
return array ( 'invalid-undelete' );
}
diff --git a/includes/Model/Workflow.php b/includes/Model/Workflow.php
index c4cee2b..5497c63 100644
--- a/includes/Model/Workflow.php
+++ b/includes/Model/Workflow.php
@@ -365,18 +365,11 @@
* @return array Array of arrays of the arguments to wfMessage to
explain permissions problems.
*/
public function getPermissionErrors( $permission, $user ) {
- $title = $this->getArticleTitle();
+ $title = $this->type === 'topic' ? $this->getOwnerTitle() :
$this->getArticleTitle();
$errors = $title->getUserPermissionsErrors( $permission, $user
);
if ( count( $errors ) ) {
return $errors;
- }
-
- if ( $this->type === 'topic' ) {
- $errors =
$this->getOwnerTitle()->getUserPermissionsErrors( $permission, $user );
- if ( count( $errors ) ) {
- return $errors;
- }
}
return array();
diff --git a/includes/SubmissionHandler.php b/includes/SubmissionHandler.php
index c9652d4..c2ff957 100644
--- a/includes/SubmissionHandler.php
+++ b/includes/SubmissionHandler.php
@@ -99,6 +99,10 @@
if ( $errorMsgArgs ) {
$msg->params( $errorMsgArgs );
}
+ // I guess this is the "user block" meaning of
'block'. If
+ // so, this is misleading, since it could be
protection,
+ // etc. In theory, something could be using it
since it's
+ // exposed to the API, but very unlikely...
reset( $interestedBlocks )->addError( 'block',
$msg );
}
return array();
diff --git a/modules/flow/ui/widgets/mw.flow.ui.BoardDescriptionWidget.js
b/modules/flow/ui/widgets/mw.flow.ui.BoardDescriptionWidget.js
index 4a33f6c..d6f71d3 100644
--- a/modules/flow/ui/widgets/mw.flow.ui.BoardDescriptionWidget.js
+++ b/modules/flow/ui/widgets/mw.flow.ui.BoardDescriptionWidget.js
@@ -245,7 +245,7 @@
.then( null, function ( errorCode, errorObj ) {
widget.captcha.update( errorCode, errorObj );
if ( !widget.captcha.isRequired() ) {
- widget.error.setLabel( errorObj.error
&& errorObj.error.info || errorObj.exception );
+ widget.error.setLabel( new
OO.ui.HtmlSnippet( errorObj.error && errorObj.error.info || errorObj.exception
) );
widget.error.toggle( true );
}
} )
diff --git a/modules/flow/ui/widgets/mw.flow.ui.EditPostWidget.js
b/modules/flow/ui/widgets/mw.flow.ui.EditPostWidget.js
index 7333394..388201d 100644
--- a/modules/flow/ui/widgets/mw.flow.ui.EditPostWidget.js
+++ b/modules/flow/ui/widgets/mw.flow.ui.EditPostWidget.js
@@ -142,7 +142,7 @@
.then( null, function ( errorCode, errorObj ) {
widget.captcha.update( errorCode, errorObj );
if ( !widget.captcha.isRequired() ) {
- widget.error.setLabel( errorObj.error
&& errorObj.error.info || errorObj.exception );
+ widget.error.setLabel( new
OO.ui.HtmlSnippet( errorObj.error && errorObj.error.info || errorObj.exception
) );
widget.error.toggle( true );
}
} )
diff --git a/modules/flow/ui/widgets/mw.flow.ui.EditTopicSummaryWidget.js
b/modules/flow/ui/widgets/mw.flow.ui.EditTopicSummaryWidget.js
index 982d4c2..846ee2d 100644
--- a/modules/flow/ui/widgets/mw.flow.ui.EditTopicSummaryWidget.js
+++ b/modules/flow/ui/widgets/mw.flow.ui.EditTopicSummaryWidget.js
@@ -143,7 +143,7 @@
.then( null, function ( errorCode, errorObj ) {
widget.captcha.update( errorCode, errorObj );
if ( !widget.captcha.isRequired() ) {
- widget.error.setLabel( errorObj.error
&& errorObj.error.info || errorObj.exception );
+ widget.error.setLabel( new
OO.ui.HtmlSnippet( errorObj.error && errorObj.error.info || errorObj.exception
) );
widget.error.toggle( true );
}
} )
diff --git a/modules/flow/ui/widgets/mw.flow.ui.NewTopicWidget.js
b/modules/flow/ui/widgets/mw.flow.ui.NewTopicWidget.js
index 3ccb35b..406636b 100644
--- a/modules/flow/ui/widgets/mw.flow.ui.NewTopicWidget.js
+++ b/modules/flow/ui/widgets/mw.flow.ui.NewTopicWidget.js
@@ -176,7 +176,7 @@
.then( null, function ( errorCode, errorObj ) {
widget.captcha.update( errorCode, errorObj );
if ( !widget.captcha.isRequired() ) {
- widget.error.setLabel( errorObj.error
&& errorObj.error.info || errorObj.exception );
+ widget.error.setLabel( new
OO.ui.HtmlSnippet( errorObj.error && errorObj.error.info || errorObj.exception
) );
widget.error.toggle( true );
}
} )
diff --git a/modules/flow/ui/widgets/mw.flow.ui.ReplyWidget.js
b/modules/flow/ui/widgets/mw.flow.ui.ReplyWidget.js
index de2a8ae..8d06287 100644
--- a/modules/flow/ui/widgets/mw.flow.ui.ReplyWidget.js
+++ b/modules/flow/ui/widgets/mw.flow.ui.ReplyWidget.js
@@ -131,7 +131,7 @@
.then( null, function ( errorCode, errorObj ) {
widget.captcha.update( errorCode, errorObj );
if ( !widget.captcha.isRequired() ) {
- widget.error.setLabel( errorObj.error
&& errorObj.error.info || errorObj.exception );
+ widget.error.setLabel( new
OO.ui.HtmlSnippet( errorObj.error && errorObj.error.info || errorObj.exception
) );
widget.error.toggle( true );
}
diff --git a/modules/flow/ui/widgets/mw.flow.ui.TopicTitleWidget.js
b/modules/flow/ui/widgets/mw.flow.ui.TopicTitleWidget.js
index 1a5637c..281ab2f 100644
--- a/modules/flow/ui/widgets/mw.flow.ui.TopicTitleWidget.js
+++ b/modules/flow/ui/widgets/mw.flow.ui.TopicTitleWidget.js
@@ -118,7 +118,7 @@
function ( errorCode, errorObj ) {
widget.captcha.update( errorCode, errorObj );
if ( !widget.captcha.isRequired() ) {
- widget.error.setLabel( errorObj.error
&& errorObj.error.info || errorObj.exception );
+ widget.error.setLabel( new
OO.ui.HtmlSnippet( errorObj.error && errorObj.error.info || errorObj.exception
) );
widget.error.toggle( true );
}
}
diff --git a/modules/mw.flow.Initializer.js b/modules/mw.flow.Initializer.js
index 8c04f9d..3bc0e49 100644
--- a/modules/mw.flow.Initializer.js
+++ b/modules/mw.flow.Initializer.js
@@ -878,7 +878,7 @@
new
OO.ui.HtmlSnippet( errorObj.error.info )
);
} else {
-
editor.error.setLabel( errorObj.error && errorObj.error.info ||
errorObj.exception );
+
editor.error.setLabel( new OO.ui.HtmlSnippet( errorObj.error &&
errorObj.error.info || errorObj.exception ) );
}
editor.error.toggle(
true );
@@ -939,7 +939,7 @@
new
OO.ui.HtmlSnippet( errorObj.error.info )
);
} else {
- error.setLabel(
errorObj.error && errorObj.error.info || errorObj.exception );
+ error.setLabel(
new OO.ui.HtmlSnippet( errorObj.error && errorObj.error.info ||
errorObj.exception ) );
}
editor.popPending();
}
--
To view, visit https://gerrit.wikimedia.org/r/279567
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0ca448380491400400f9bf35f812b4f73f67d26f
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Mattflaschen <mflasc...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
