Catrope has uploaded a new change for review.
https://gerrit.wikimedia.org/r/301056
Change subject: Reduce mayhem caused by ?oldid=currentRevId
......................................................................
Reduce mayhem caused by ?oldid=currentRevId
If you viewed a page with an ?oldid= query parameter set to the ID
of the current revision, some parts of VE would believe we were
in oldid mode (because there's an oldid present), but others
wouldn't (because the revid we're editing equals the newest revid).
This caused bugs when opening the editor a second time after saving
(which is normally impossible to do after an oldid-mode edit, because
we navigate to a new page after an oldid save, but we don't do that
in this case).
Ensure that:
* The internal state of DesktopArticleTarget is updated correctly
after saving in this case
* The ?oldid= parameter is removed from the URL after saving
* DesktopArticleTarget.init doesn't preload the article HTML
on a second/subsequent editor load: this causes issues because
it caches the oldid, and generally speaking the Target's internal
state is not considered
Bug: T141330
Change-Id: I74034328797c59f7249f1f6f4f53a92ee1c26334
---
M modules/ve-mw/init/targets/ve.init.mw.DesktopArticleTarget.init.js
M modules/ve-mw/init/targets/ve.init.mw.DesktopArticleTarget.js
2 files changed, 31 insertions(+), 14 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/56/301056/1
diff --git a/modules/ve-mw/init/targets/ve.init.mw.DesktopArticleTarget.init.js
b/modules/ve-mw/init/targets/ve.init.mw.DesktopArticleTarget.init.js
index 64c6d4e..06cbf5f 100644
--- a/modules/ve-mw/init/targets/ve.init.mw.DesktopArticleTarget.init.js
+++ b/modules/ve-mw/init/targets/ve.init.mw.DesktopArticleTarget.init.js
@@ -25,6 +25,7 @@
tabPreference, userPrefEnabled, userPrefPreferShow,
initialWikitext, oldid,
onlyTabIsVE,
active = false,
+ targetLoaded = false,
progressStep = 0,
progressSteps = [
[ 30, 3000 ],
@@ -154,6 +155,7 @@
}
} );
target.setContainer( $( '#content' ) );
+ targetLoaded = true;
return target;
}, function ( e ) {
mw.log.warn( 'VisualEditor failed to
load: ' + e );
@@ -205,19 +207,25 @@
* @param {boolean} [modified] The page was been modified before
loading (e.g. in source mode)
*/
function activateTarget( targetPromise, modified ) {
- // The TargetLoader module is loaded in the bottom queue, so it
should have been
- // requested already but it might not have finished loading yet
- var dataPromise = mw.loader.using(
'ext.visualEditor.targetLoader' )
- .then( function () {
- return mw.libs.ve.targetLoader.requestPageData(
- mw.config.get( 'wgRelevantPageName' ),
- oldid,
- 'mwTarget', //
ve.init.mw.DesktopArticleTarget.static.name
- modified
- );
- } )
- .done( incrementLoadingProgress )
- .fail( handleLoadFailure );
+ var dataPromise;
+ // Only call requestPageData early if the target object isn't
there yet.
+ // If the target object is there, this is a second or
subsequent load, and the
+ // internal state of the target object can influence the load
request.
+ if ( !targetLoaded ) {
+ // The TargetLoader module is loaded in the bottom
queue, so it should have been
+ // requested already but it might not have finished
loading yet
+ dataPromise = mw.loader.using(
'ext.visualEditor.targetLoader' )
+ .then( function () {
+ return
mw.libs.ve.targetLoader.requestPageData(
+ mw.config.get(
'wgRelevantPageName' ),
+ oldid,
+ 'mwTarget', //
ve.init.mw.DesktopArticleTarget.static.name
+ modified
+ );
+ } )
+ .done( incrementLoadingProgress )
+ .fail( handleLoadFailure );
+ }
setEditorPreference( 'visualeditor' );
diff --git a/modules/ve-mw/init/targets/ve.init.mw.DesktopArticleTarget.js
b/modules/ve-mw/init/targets/ve.init.mw.DesktopArticleTarget.js
index 99db24a..28471b8 100644
--- a/modules/ve-mw/init/targets/ve.init.mw.DesktopArticleTarget.js
+++ b/modules/ve-mw/init/targets/ve.init.mw.DesktopArticleTarget.js
@@ -892,8 +892,12 @@
// If we were explicitly editing an older version, make sure we
won't
// load the same old version again, now that we've saved the
next edit
// will be against the latest version.
- // TODO: What about oldid in the url?
+ // If there is an ?oldid= parameter in the URL, this will cause
restorePage() to remove it.
this.restoring = false;
+
+ // Clear requestedRevId in case it was set by a retry or
something; after saving
+ // we don't want to go back into oldid mode anyway
+ this.requestedRevId = undefined;
if ( newid !== undefined ) {
mw.config.set( {
@@ -901,6 +905,7 @@
wgRevisionId: newid
} );
this.revid = newid;
+ this.currentRevisionId = newid;
}
// Update module JS config values and notify ResourceLoader of
any new
@@ -1246,6 +1251,10 @@
delete uri.query.action;
mw.config.set( 'wgAction', 'view' );
}
+ if ( 'oldid' in uri.query && !this.restoring ) {
+ // We have an oldid in the query string but it's the
most recent one, so remove it
+ delete uri.query.oldid;
+ }
// If there are any other query parameters left, re-use that
uri object.
// Otherwise use the canonical style view url (T44553, T102363).
--
To view, visit https://gerrit.wikimedia.org/r/301056
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I74034328797c59f7249f1f6f4f53a92ee1c26334
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
