jenkins-bot has submitted this change and it was merged.
Change subject: Actually pass oldid parameter when serializing
......................................................................
Actually pass oldid parameter when serializing
Apparently Parsoid has always required this, and we've never sent it,
yet somehow the code in production works. This may well be the cause
of some of the selser issues we saw after the deployment attempt in
January.
Made oldid a required parameter in the API module, and default it to 0.
When we get 0, we translate that to the empty string for Parsoid's
benefit. We also need to explicitly get wgCurRevisionId in
ViewPageTarget, and that's also 0 on new pages.
Change-Id: I3a55025246014cd74e15d6d5b6c4ede7b823e5df
---
M ApiVisualEditor.php
M modules/ve/init/mw/ve.init.mw.Target.js
2 files changed, 20 insertions(+), 15 deletions(-)
Approvals:
Trevor Parscal: Looks good to me, approved
GWicke: Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/ApiVisualEditor.php b/ApiVisualEditor.php
index 1084b65..ed258f2 100644
--- a/ApiVisualEditor.php
+++ b/ApiVisualEditor.php
@@ -13,10 +13,8 @@
global $wgVisualEditorParsoidURL, $wgVisualEditorParsoidPrefix,
$wgVisualEditorParsoidTimeout;
if ( $title->exists() ) {
- if ( !isset( $parserParams['oldid'] ) ) {
- // Don't allow race condition where the latest
revision ID changes while we are waiting
- // for a response from Parsoid
- $parserParams['oldid'] =
$title->getLatestRevId();
+ if ( $parserParams['oldid'] === 0 ) {
+ $parserParams['oldid'] = ''; // Parsoid wants
empty string rather than zero
}
$revision = Revision::newFromId( $parserParams['oldid']
);
if ( $revision === null ) {
@@ -67,15 +65,19 @@
);
}
- protected function postHTML( $title, $html ) {
+ protected function postHTML( $title, $html, $parserParams ) {
global $wgVisualEditorParsoidURL, $wgVisualEditorParsoidPrefix,
$wgVisualEditorParsoidTimeout;
+ if ( $parserParams['oldid'] === 0 ) {
+ $parserParams['oldid'] = '';
+ }
return Http::post(
$wgVisualEditorParsoidURL . '/' .
$wgVisualEditorParsoidPrefix .
'/' . urlencode( $title->getPrefixedDBkey() ),
array(
'postData' => array( 'content' => $html ),
- 'timeout' => $wgVisualEditorParsoidTimeout
+ 'timeout' => $wgVisualEditorParsoidTimeout,
+ 'oldid' => $parserParams['oldid']
)
);
}
@@ -184,10 +186,7 @@
$page->getNamespace(), 'novenamespace' );
}
- $parserParams = array();
- if ( is_numeric( $params['oldid'] ) ) {
- $parserParams['oldid'] = intval( $params['oldid'] );
- }
+ $parserParams = array( 'oldid' => $params['oldid'] );
if ( $params['paction'] === 'parse' ) {
$parsed = $this->getHTML( $page, $parserParams );
@@ -211,14 +210,14 @@
if ( $params['html'] === null ) {
$this->dieUsageMsg( 'missingparam', 'html' );
}
- $serialized = array( 'content' => $this->postHTML(
$page, $params['html'] ) );
+ $serialized = array( 'content' => $this->postHTML(
$page, $params['html'], $parserParams ) );
if ( $serialized === false ) {
$this->dieUsage( 'Error contacting the Parsoid
server', 'parsoidserver' );
} else {
$result = array_merge( array( 'result' =>
'success' ), $serialized );
}
} elseif ( $params['paction'] === 'save' || $params['paction']
=== 'diff' ) {
- $wikitext = $this->postHTML( $page, $params['html'] );
+ $wikitext = $this->postHTML( $page, $params['html'],
$parserParams );
if ( $wikitext === false ) {
$this->dieUsage( 'Error contacting the Parsoid
server', 'parsoidserver' );
@@ -275,7 +274,11 @@
),
'basetimestamp' => null,
'starttimestamp' => null,
- 'oldid' => null,
+ 'oldid' => array(
+ ApiBase::PARAM_REQUIRED => true,
+ ApiBase::PARAM_TYPE => 'integer',
+ ApiBase::PARAM_DFLT => 0
+ ),
'minor' => null,
'watch' => null,
'html' => null,
@@ -307,7 +310,7 @@
return array(
'page' => 'The page to perform actions on.',
'paction' => 'Action to perform',
- 'oldid' => 'The revision number to use.',
+ 'oldid' => 'The revision number to use. If zero, the
empty string is passed to Parsoid to indicate new page creation.',
'minor' => 'Flag for minor edit.',
'html' => 'HTML to send to parsoid in exchange for
wikitext',
'summary' => 'Edit summary',
diff --git a/modules/ve/init/mw/ve.init.mw.Target.js
b/modules/ve/init/mw/ve.init.mw.Target.js
index 05abd6f..858147a 100644
--- a/modules/ve/init/mw/ve.init.mw.Target.js
+++ b/modules/ve/init/mw/ve.init.mw.Target.js
@@ -25,7 +25,7 @@
// Properties
this.pageName = pageName;
this.pageExists = mw.config.get( 'wgArticleId', 0 ) !== 0;
- this.oldid = revision || '';
+ this.oldid = revision || mw.config.get( 'wgCurRevisionId' );
this.editToken = mw.user.tokens.get( 'editToken' );
this.apiUrl = mw.util.wikiScript( 'api' );
this.submitUrl = ( new mw.Uri( mw.util.wikiGetlink( this.pageName ) ) )
@@ -462,6 +462,7 @@
'action': 'visualeditor',
'paction': 'diff',
'page': this.pageName,
+ 'oldid': this.oldid,
'html': doc.body.innerHTML, // TODO make this send the
whole document in the future
// TODO: API required editToken, though not relevant
for diff
'token': this.editToken
@@ -552,6 +553,7 @@
'paction': 'serialize',
'html': doc.body.innerHTML, // TODO make this send the
whole document in the future
'page': this.pageName,
+ 'oldid': this.oldid,
'token': this.editToken,
'format': 'json'
},
--
To view, visit https://gerrit.wikimedia.org/r/59354
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3a55025246014cd74e15d6d5b6c4ede7b823e5df
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: GWicke <[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
